[PATCH] zram: compression algorithms name use static fields

[PATCH] zram: compression algorithms name use static fields

[gao xu]

Currently, zram dynamically allocates memory for compressor algorithm names
when they are set by the user. This requires careful memory management,
including explicit `kfree` calls and special handling to avoid freeing
statically defined default compressor names.

This patch refactors the way zram handles compression algorithm names.
Instead of storing dynamically allocated copies, `zram->comp_algs` will now
store pointers directly to the static name strings defined within the
`zcomp_ops` backend structures, thereby removing the need for conditional
`kfree` calls.

Signed-off-by: gao xu <gaoxu2@honor.com>
---
 drivers/block/zram/zcomp.c    |  9 +++++++--
 drivers/block/zram/zcomp.h    |  2 +-
 drivers/block/zram/zram_drv.c | 19 ++++++-------------
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index a771a8ecc..974c46918 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -84,9 +84,14 @@ static const struct zcomp_ops *lookup_backend_ops(const char *comp)
 	return backends[i];
 }
 
-bool zcomp_available_algorithm(const char *comp)
+const char *zcomp_lookup_backend_name(const char *comp)
 {
-	return lookup_backend_ops(comp) != NULL;
+	const struct zcomp_ops *backend = lookup_backend_ops(comp);
+
+	if (backend)
+		return backend->name;
+
+	return NULL;
 }
 
 /* show available compressors */
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index eacfd3f7d..81a0f3f6f 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -80,7 +80,7 @@ struct zcomp {
 int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node);
 int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node);
 ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at);
-bool zcomp_available_algorithm(const char *comp);
+const char *zcomp_lookup_backend_name(const char *comp);
 
 struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params);
 void zcomp_destroy(struct zcomp *comp);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index bca33403f..2472d3e5c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1637,10 +1637,6 @@ static void zram_debugfs_unregister(struct zram *zram) {};
 
 static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg)
 {
-	/* Do not free statically defined compression algorithms */
-	if (zram->comp_algs[prio] != default_compressor)
-		kfree(zram->comp_algs[prio]);
-
 	zram->comp_algs[prio] = alg;
 }
 
@@ -1648,6 +1644,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
 {
 	char *compressor;
 	size_t sz;
+	const char *alg;
 
 	sz = strlen(buf);
 	if (sz >= ZRAM_MAX_ALGO_NAME_SZ)
@@ -1661,10 +1658,10 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
 	if (sz > 0 && compressor[sz - 1] == '\n')
 		compressor[sz - 1] = 0x00;
 
-	if (!zcomp_available_algorithm(compressor)) {
-		kfree(compressor);
+	alg = zcomp_lookup_backend_name(compressor);
+	kfree(compressor);
+	if (!alg)
 		return -EINVAL;
-	}
 
 	guard(rwsem_write)(&zram->dev_lock);
 	if (init_done(zram)) {
@@ -1673,7 +1670,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
 		return -EBUSY;
 	}
 
-	comp_algorithm_set(zram, prio, compressor);
+	comp_algorithm_set(zram, prio, alg);
 	return 0;
 }
 
@@ -2851,12 +2848,8 @@ static void zram_destroy_comps(struct zram *zram)
 		zram->num_active_comps--;
 	}
 
-	for (prio = ZRAM_PRIMARY_COMP; prio < ZRAM_MAX_COMPS; prio++) {
-		/* Do not free statically defined compression algorithms */
-		if (zram->comp_algs[prio] != default_compressor)
-			kfree(zram->comp_algs[prio]);
+	for (prio = ZRAM_PRIMARY_COMP; prio < ZRAM_MAX_COMPS; prio++)
 		zram->comp_algs[prio] = NULL;
-	}
 
 	zram_comp_params_reset(zram);
 }
--

Re: [PATCH] zram: compression algorithms name use static fields

[Sergey Senozhatsky]

On (26/02/26 07:50), gao xu wrote:
[..]
> @@ -1648,6 +1644,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
>  {
>  	char *compressor;
>  	size_t sz;
> +	const char *alg;
>  
>  	sz = strlen(buf);
>  	if (sz >= ZRAM_MAX_ALGO_NAME_SZ)
> @@ -1661,10 +1658,10 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf)
>  	if (sz > 0 && compressor[sz - 1] == '\n')
>  		compressor[sz - 1] = 0x00;
>  
> -	if (!zcomp_available_algorithm(compressor)) {
> -		kfree(compressor);
> +	alg = zcomp_lookup_backend_name(compressor);
> +	kfree(compressor);

I guess we don't really need to kstrdup() the compressor name?  Lookup
uses sysfs_streq(), which should take care of terminating \n, so I assume
we can just pass `buf` to zcomp_lookup_backend_name() directly?

I guess we also can get rid of `[sz - 1] == '\n'` thing?

Re: [PATCH] zram: compression algorithms name use static fields

[Sergey Senozhatsky]

On (26/02/26 07:50), gao xu wrote:
> Subject: [PATCH] zram: compression algorithms name use static fields

Can you also change the order of the words in the subject line in v2?
Perhaps, to something like
	zram: use statically allocated compression algorithms names
or something along those lines.  The current subject line is a bit
hard to parse.

RE: [PATCH] zram: compression algorithms name use static fields

[gao xu]

> 
> On (26/02/26 07:50), gao xu wrote:
> [..]
> > @@ -1648,6 +1644,7 @@ static int __comp_algorithm_store(struct zram
> *zram, u32 prio, const char *buf)
> >  {
> >  	char *compressor;
> >  	size_t sz;
> > +	const char *alg;
> >
> >  	sz = strlen(buf);
> >  	if (sz >= ZRAM_MAX_ALGO_NAME_SZ)
> > @@ -1661,10 +1658,10 @@ static int __comp_algorithm_store(struct zram
> *zram, u32 prio, const char *buf)
> >  	if (sz > 0 && compressor[sz - 1] == '\n')
> >  		compressor[sz - 1] = 0x00;
> >
> > -	if (!zcomp_available_algorithm(compressor)) {
> > -		kfree(compressor);
> > +	alg = zcomp_lookup_backend_name(compressor);
> > +	kfree(compressor);
> 
> I guess we don't really need to kstrdup() the compressor name?  Lookup
> uses sysfs_streq(), which should take care of terminating \n, so I assume
> we can just pass `buf` to zcomp_lookup_backend_name() directly?
> 
> I guess we also can get rid of `[sz - 1] == '\n'` thing?
I've verified your suggested changes and haven't found any issues. 
I will incorporate this modification into the v2 version.

RE: [PATCH] zram: compression algorithms name use static fields

[gao xu]

> 
> Can you also change the order of the words in the subject line in v2?
> Perhaps, to something like
> 	zram: use statically allocated compression algorithms names or something
> along those lines.  The current subject line is a bit hard to parse.
Got it, I'll adjust the subject line in v2.