Re: [PATCH 2/2] zram: Use a custom key for each zram object.

On (26/07/24 12:24), Sergey Senozhatsky wrote:
> >  static bool zram_meta_alloc(struct zram *zram, u64 disksize)
> > @@ -1999,7 +1993,8 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
> >  	if (!huge_class_size)
> >  		huge_class_size = zs_huge_class_size(zram->mem_pool);
> >  
> > -	slot_lock_init(zram);
> > +	lockdep_register_key(&zram->table_lock_key);
> > +	lockdep_init_map(&zram->table_lock_map, "zram->table[index].lock", &zram->table_lock_key, 0);
> 
> So I wonder if we can give distinct name, given that it's per-device
> now: "zram%d->table[index].lock" and use zram->disk->first_minor (which
> is a unique device id).

Probably (see below) is not worth it.  All those ifdef-s...  We used static
name before and didn't have problems with it.  So let's just keep doing so
(IOW let's keep not having problems ;))

// completely untested
----

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4bfe63a5225d..080012e83f86 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1956,6 +1956,31 @@ static ssize_t debug_stat_show(struct device *dev,
 	return ret;
 }
 
+static void zram_lockdep_map_release(struct zram *zram)
+{
+#ifdef CONFIG_LOCKDEP
+	lockdep_unregister_key(&zram->table_lock_key);
+	kfree(zram->table_lock_name);
+#endif
+}
+
+static int zram_lockdep_map_init(struct zram *zram)
+{
+#ifdef CONFIG_LOCKDEP
+	zram->table_lock_name = kasprintf(GFP_KERNEL,
+					  "zram%d->table[index].lock",
+					  zram->disk->first_minor);
+	if (!zram->table_lock_name)
+		return -ENOMEM;
+
+	lockdep_register_key(&zram->table_lock_key);
+	lockdep_init_map(&zram->table_lock_map, zram->table_lock_name,
+			 &zram->table_lock_key, 0);
+#endif
+	return 0;
+}
+
 static void zram_meta_free(struct zram *zram, u64 disksize)
 {
 	size_t num_pages = disksize >> PAGE_SHIFT;
@@ -1971,7 +1996,7 @@ static void zram_meta_free(struct zram *zram, u64 disksize)
 	zs_destroy_pool(zram->mem_pool);
 	vfree(zram->table);
 	zram->table = NULL;
-	lockdep_unregister_key(&zram->table_lock_key);
+	zram_lockdep_map_release(zram);
 }
 
 static bool zram_meta_alloc(struct zram *zram, u64 disksize)
@@ -1990,12 +2015,16 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
 		return false;
 	}
 
+	if (zram_lockdep_map_init(zram)) {
+		zs_destroy_pool(zram->mem_pool);
+		vfree(zram->table);
+		zram->table = NULL;
+		return false;
+	}
+
 	if (!huge_class_size)
 		huge_class_size = zs_huge_class_size(zram->mem_pool);
 
-	lockdep_register_key(&zram->table_lock_key);
-	lockdep_init_map(&zram->table_lock_map, "zram->table[index].lock", &zram->table_lock_key, 0);
-
 	return true;
 }
 
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 4fddc582f3b8..377832477588 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -106,8 +106,6 @@ struct zram_stats {
 
 struct zram {
 	struct zram_table_entry *table;
-	struct lockdep_map table_lock_map;
-	struct lock_class_key table_lock_key;
 	struct zs_pool *mem_pool;
 	struct zcomp *comps[ZRAM_MAX_COMPS];
 	struct zcomp_params params[ZRAM_MAX_COMPS];
@@ -143,5 +141,11 @@ struct zram {
 #ifdef CONFIG_ZRAM_MEMORY_TRACKING
 	struct dentry *debugfs_dir;
 #endif
+
+	struct lockdep_map table_lock_map;
+	struct lock_class_key table_lock_key;
+#ifdef CONFIG_LOCKDEP
+	char *table_lock_name;
+#endif
 };
 #endif