zram: Replace pr_* with dev_*

PATCH In review Salah Triki 4 participants 17 messages Aug 6, 2015 – Aug 10, 2015

Subsystems (2)

Block layer zram

lore

Share & tools

lore

Salah Triki

author

2015-08-06 22:57 UTC #0 lore raw

cover zram: Replace pr_* with dev_*

This patchset replaces pr_* with dev_*. dev_* attach kernel messages to the right device. In addition, patchs 1 and 2 add to messages the values of variables that trigger errors.

Salah Triki (3):
zram: Replace pr_info with dev_info in max_comp_streams_store
zram: Replace pr_info with dev_info in comp_algorithm_store
zram: Replace pr_* with dev_*

 drivers/block/zram/zram_drv.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

Signature · 1 line

1.9.1

Salah Triki

author

2015-08-06 23:04 UTC #1 lore raw

PATCH 1/3 zram: Replace pr_info with dev_info in max_comp_streams_store

dev_info attachs the message to the device. And, the max compression streams value, that triggers error, is added to the message.

---
 drivers/block/zram/zram_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/block/zram/zram_drv.c +2 -1

index fb655e8..bb284db 100644
@@ -333,7 +333,8 @@ static ssize_t max_comp_streams_store(struct device *dev,
 	down_write(&zram->init_lock);
 	if (init_done(zram)) {
 		if (!zcomp_set_max_streams(zram->comp, num)) {
-			pr_info("Cannot change max compression streams\n");
+			dev_info(dev, "Cannot change max compression streams to %d\n",
+				num);
 			ret = -EINVAL;
 			goto out;
 		}

Signature · 1 line

1.9.1

Salah Triki

author

2015-08-06 23:04 UTC #2 lore raw

PATCH 2/3 zram: Replace pr_info with dev_info in comp_algorithm_store

In reply to Salah Triki

dev_info attachs the message to the device. And, the algorithm's name, that triggers the error, is added to the message.

---
 drivers/block/zram/zram_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/block/zram/zram_drv.c +2 -1

index bb284db..1c2f8b9 100644
@@ -369,7 +369,8 @@ static ssize_t comp_algorithm_store(struct device *dev,
 	down_write(&zram->init_lock);
 	if (init_done(zram)) {
 		up_write(&zram->init_lock);
-		pr_info("Can't change algorithm for initialized device\n");
+		dev_info(dev, "Can't change algorithm to %s for initialized device\n",
+			 buf);
 		return -EBUSY;
 	}
 	strlcpy(zram->compressor, buf, sizeof(zram->compressor));

Signature · 1 line

1.9.1

Salah Triki

author

2015-08-06 23:04 UTC #3 lore raw

PATCH 3/3 zram: Replace pr_* with dev_*

In reply to Salah Triki

dev_* attach the messages to the devices.

---
 drivers/block/zram/zram_drv.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

drivers/block/zram/zram_drv.c +16 ~~-12~~

index 1c2f8b9..73f3453 100644
@@ -589,7 +589,8 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
 
 	/* Should NEVER happen. Return bio error if it does. */
 	if (unlikely(ret)) {
-		pr_err("Decompression failed! err=%d, page=%u\n", ret, index);
+		dev_err(disk_to_dev(zram->disk), "Decompression failed! err=%d, page=%u\n",
+			ret, index);
 		return ret;
 	}

@@ -623,7 +624,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
 		uncmem = user_mem;
 
 	if (!uncmem) {
-		pr_info("Unable to allocate temp memory\n");
+		dev_info(disk_to_dev(zram->disk), "Unable to allocate temp memory\n");
 		ret = -ENOMEM;
 		goto out_cleanup;
 	}

@@ -708,7 +709,8 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 	}
 
 	if (unlikely(ret)) {
-		pr_err("Compression failed! err=%d\n", ret);
+		dev_err(disk_to_dev(zram->disk), "Compression failed! err=%d\n",
+			ret);
 		goto out;
 	}
 	src = zstrm->buffer;

@@ -720,7 +722,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 
 	handle = zs_malloc(meta->mem_pool, clen);
 	if (!handle) {
-		pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
+		dev_info(disk_to_dev(zram->disk), "Error allocating memory for compressed page: %u, size=%zu\n",
 			index, clen);
 		ret = -ENOMEM;
 		goto out;

@@ -1039,15 +1041,15 @@ static ssize_t disksize_store(struct device *dev,
 
 	comp = zcomp_create(zram->compressor, zram->max_comp_streams);
 	if (IS_ERR(comp)) {
-		pr_info("Cannot initialise %s compressing backend\n",
-				zram->compressor);
+		dev_info(dev, "Cannot initialise %s compressing backend\n",
+			zram->compressor);
 		err = PTR_ERR(comp);
 		goto out_free_meta;
 	}
 
 	down_write(&zram->init_lock);
 	if (init_done(zram)) {
-		pr_info("Cannot change disksize for initialized device\n");
+		dev_info(dev, "Cannot change disksize for initialized device\n");
 		err = -EBUSY;
 		goto out_destroy_comp;
 	}

@@ -1206,7 +1208,7 @@ static int zram_add(void)
 
 	queue = blk_alloc_queue(GFP_KERNEL);
 	if (!queue) {
-		pr_err("Error allocating disk queue for device %d\n",
+		dev_err(disk_to_dev(zram->disk), "Error allocating disk queue for device %d\n",
 			device_id);
 		ret = -ENOMEM;
 		goto out_free_idr;

@@ -1217,7 +1219,7 @@ static int zram_add(void)
 	/* gendisk structure */
 	zram->disk = alloc_disk(1);
 	if (!zram->disk) {
-		pr_warn("Error allocating disk structure for device %d\n",
+		dev_warn(disk_to_dev(zram->disk), "Error allocating disk structure for device %d\n",
 			device_id);
 		ret = -ENOMEM;
 		goto out_free_queue;

@@ -1266,14 +1268,15 @@ static int zram_add(void)
 	ret = sysfs_create_group(&disk_to_dev(zram->disk)->kobj,
 				&zram_disk_attr_group);
 	if (ret < 0) {
-		pr_warn("Error creating sysfs group");
+		dev_warn(disk_to_dev(zram->disk), "Error creating sysfs group");
 		goto out_free_disk;
 	}
 	strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor));
 	zram->meta = NULL;
 	zram->max_comp_streams = 1;
 
-	pr_info("Added device: %s\n", zram->disk->disk_name);
+	dev_info(disk_to_dev(zram->disk), "Added device: %s\n",
+		zram->disk->disk_name);
 	return device_id;
 
 out_free_disk:

@@ -1321,7 +1324,8 @@ static int zram_remove(struct zram *zram)
 	zram_reset_device(zram);
 	bdput(bdev);
 
-	pr_info("Removed device: %s\n", zram->disk->disk_name);
+	dev_info(disk_to_dev(zram->disk), "Removed device: %s\n",
+		zram->disk->disk_name);
 
 	idr_remove(&zram_index_idr, zram->disk->first_minor);
 	blk_cleanup_queue(zram->disk->queue);

Signature · 1 line

1.9.1

Sergey Senozhatsky

2015-08-07 00:04 UTC #4 lore raw

In reply to Salah Triki

Quoted 4 lines · Salah Triki

This patchset replaces pr_* with dev_*. dev_* attach kernel messages to the right device. In addition, patchs 1 and 2 add to messages the values of variables that trigger errors.

Hi,

I'd prefer to leave the messages the way they are. Changing anything visible to user space (api, eror codes, error messages, etc.) is a very risky business. You change the format of error messages and it smells like a big NO-NO.

'zram: Cannot initialise lzo compressing backend' --> 'block zram0: Cannot initialise lzo compressing backend'

And there are even more dramatic changes:
"Cannot change max compression streams\n" --> "Cannot change max compression streams to %d\n"

"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"

People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.

This potentially breaks things in user space. So, I NACK the change set. Thanks.

Minchan, any opinion?

-ss

Quoted 11 lines

Salah Triki (3):
zram: Replace pr_info with dev_info in max_comp_streams_store
zram: Replace pr_info with dev_info in comp_algorithm_store
zram: Replace pr_* with dev_*
 drivers/block/zram/zram_drv.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
Signature · 1 line
1.9.1

Joe Perches

2015-08-07 01:17 UTC #5 lore raw

Quoted 4 lines · Sergey Senozhatsky

Quoted 3 lines · Salah Triki
This patchset replaces pr_* with dev_*. dev_* attach kernel messages to the right device. In addition, patchs 1 and 2 add to messages the values of variables that trigger errors.

[]

Quoted 13 lines

I'd prefer to leave the messages the way they are. Changing anything visible to user space (api, eror codes, error messages, etc.) is a very risky business. You change the format of error messages and it smells like a big NO-NO.
'zram: Cannot initialise lzo compressing backend' --> 'block zram0: Cannot initialise lzo compressing backend'
And there are even more dramatic changes:
"Cannot change max compression streams\n" --> "Cannot change max compression streams to %d\n"

Quoted 6 lines

"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"
People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.

That's not true at all.

Using grep on dmesg is specifically _not_ guaranteed to remain stable between kernel versions.

Sergey Senozhatsky

2015-08-07 01:41 UTC #6 lore raw

Hello Joe,

On (08/06/15 18:17), Joe Perches wrote:
[..]

Quoted 11 lines

Quoted 6 lines
"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"
People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.
That's not true at all.
Using grep on dmesg is specifically _not_ guaranteed to remain stable between kernel versions.

It depends, I guess. People do use grep after all and people don't like when things are getting changed underneath; and we don't want to do this. I think Minchan is with me here. We even didn't add some additional pr_info/pr_err noise recently because we don't want people to depend on that part.

http://lkml.iu.edu/hypermail/linux/kernel/1505.3/01759.html

Minchan Kim <[email protected]>:

|I meant if we remove the pr_err in future by some reason,
|someone might shout
|
|"No, it's ABI so if you guys removes it, it will break user interface's
|semantic". Maybe he seems to depends on parse on dmesg.
|That is not what I want.

And I saw some time ago people doing that type of thing. So I'd like to avoid unnecessary pain for zram users even if the messages are not guaranteed to remain stable between kernel releases. Just my opinion.

-ss

Joe Perches

2015-08-07 01:48 UTC #7 lore raw

Quoted 33 lines · Sergey Senozhatsky

Hello Joe,
On (08/06/15 18:17), Joe Perches wrote:
[..]
Quoted 11 lines
Quoted 6 lines
"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"
People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.
That's not true at all.
Using grep on dmesg is specifically _not_ guaranteed to remain stable between kernel versions.
It depends, I guess. People do use grep after all and people don't like when things are getting changed underneath; and we don't want to do this. I think Minchan is with me here. We even didn't add some additional pr_info/pr_err noise recently because we don't want people to depend on that part.
http://lkml.iu.edu/hypermail/linux/kernel/1505.3/01759.html
Minchan Kim <[email protected]>:
|I meant if we remove the pr_err in future by some reason,
|someone might shout
|
|"No, it's ABI so if you guys removes it, it will break user interface's
|semantic". Maybe he seems to depends on parse on dmesg.
|That is not what I want.

Quoted 3 lines

And I saw some time ago people doing that type of thing. So I'd like to avoid unnecessary pain for zram users even if the messages are not guaranteed to remain stable between kernel releases. Just my opinion.

I'm fine with you having an opinion but I'm not fine with you stating:

"We cannot change this anymore.

This potentially breaks things in user space.
So, I NACK the change set."
.
because dmesg is not an ABI.

Sergey Senozhatsky

2015-08-07 02:03 UTC #8 lore raw

On (08/06/15 18:48), Joe Perches wrote:
[..]

Quoted 3 lines

Quoted 3 lines
And I saw some time ago people doing that type of thing. So I'd like to avoid unnecessary pain for zram users even if the messages are not guaranteed to remain stable between kernel releases. Just my opinion.

[..]

Quoted 1 line

because dmesg is not an ABI.

I absolutely agree, this is debatable and controversial. I saw people grepping dmesg and they treated error messages just like errno codes or like additional info to attach to errno. Whether we like or not, but, it seems, for that cases error message is more or less part of ABI. It's fine when we change _info message, but touching _warn or _err message is a bit different thing, *I think*. Let's hear from Minchan.

-ss

Sergey Senozhatsky

2015-08-07 02:15 UTC #9 lore raw

Quoted 10 lines · Sergey Senozhatsky

[..]
Quoted 1 line
because dmesg is not an ABI.
I absolutely agree, this is debatable and controversial. I saw people grepping dmesg and they treated error messages just like errno codes or like additional info to attach to errno. Whether we like or not, but, it seems, for that cases error message is more or less part of ABI. It's fine when we change _info message, but touching _warn or _err message is a bit different thing, *I think*. Let's hear from Minchan.

A side note (not to forget).

If Minchan agress that messages are free come and go, then I'd probably like to remove this one

         meta->table = vzalloc(num_pages * sizeof(*meta->table));
         if (!meta->table) {
                 pr_err("Error allocating zram address table\n");
			^^^^^^
                 goto out_error;
         }

because __vmalloc_node_range() will warn_alloc_failed() anyway.

-ss

Minchan Kim

maintainer

2015-08-07 06:05 UTC #10 lore raw

In reply to Sergey Senozhatsky

Hello,

Quoted 25 lines · Sergey Senozhatsky

Quoted 4 lines · Salah Triki
This patchset replaces pr_* with dev_*. dev_* attach kernel messages to the right device. In addition, patchs 1 and 2 add to messages the values of variables that trigger errors.
Hi,
I'd prefer to leave the messages the way they are. Changing anything visible to user space (api, eror codes, error messages, etc.) is a very risky business. You change the format of error messages and it smells like a big NO-NO.
'zram: Cannot initialise lzo compressing backend' --> 'block zram0: Cannot initialise lzo compressing backend'
And there are even more dramatic changes:
"Cannot change max compression streams\n" --> "Cannot change max compression streams to %d\n"
"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"

Quoted 7 lines

People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.
This potentially breaks things in user space. So, I NACK the change set. Thanks.
Minchan, any opinion?

Note: I didn't read this patchset in detail so I might be wrong.

When I read description, I couldn't see what's the benefit. Please write it out.

As well, please write how you change current message. IOW, before and after.

If the benefit is not enough for me, I don't want to change.

Thanks.

Quoted 14 lines

	-ss
Quoted 11 lines
Salah Triki (3):
zram: Replace pr_info with dev_info in max_comp_streams_store
zram: Replace pr_info with dev_info in comp_algorithm_store
zram: Replace pr_* with dev_*
 drivers/block/zram/zram_drv.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
Signature · 1 line
1.9.1

Sergey Senozhatsky

2015-08-07 06:37 UTC #11 lore raw

Hello Minchan,

On (08/07/15 15:05), Minchan Kim wrote:
[..]

Quoted 23 lines

Quoted 23 lines
I'd prefer to leave the messages the way they are. Changing anything visible to user space (api, eror codes, error messages, etc.) is a very risky business. You change the format of error messages and it smells like a big NO-NO.
'zram: Cannot initialise lzo compressing backend' --> 'block zram0: Cannot initialise lzo compressing backend'
And there are even more dramatic changes:
"Cannot change max compression streams\n" --> "Cannot change max compression streams to %d\n"
"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"
People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.
This potentially breaks things in user space. So, I NACK the change set. Thanks.

Quoted 6 lines

Quoted 1 line
Minchan, any opinion?
Note: I didn't read this patchset in detail so I might be wrong.
When I read description, I couldn't see what's the benefit. Please write it out.

we now have errors like
'zram: Cannot initialise lzo compressing backend'

and they will transform into

'block zram0: Cannot initialise lzo compressing backend'

note the prefix 'zram:' became 'block zram0:'

and there are two patches (well, at least I quickly spotted only those) that change messages' text

From: "Cannot change max compression streams\n"
To: "Cannot change max compression streams to %d\n"

where %d is a supplied max_comp_stream value

From: "Can't change algorithm for initialized device\n"
To: "Can't change algorithm to %s for initialized device\n"

where %s is a supplied compression algorithm name.

as far as I can tell.

-ss

Sergey Senozhatsky

2015-08-07 06:55 UTC #12 lore raw

On (08/07/15 15:37), Sergey Senozhatsky wrote:
[..]

Quoted 8 lines

we now have errors like
'zram: Cannot initialise lzo compressing backend'
and they will transform into
'block zram0: Cannot initialise lzo compressing backend'
note the prefix 'zram:' became 'block zram0:'

but it doesn't come for free. where we had clean and nice

pr_err("Decompression failed!...
pr_info("Unable to allocate temp memory\n"...
etc...

now we have monsters

dev_err(disk_to_dev(zram->disk), "Decompression failed!... dev_info(disk_to_dev(zram->disk), "Unable to allocate temp memory\n"... etc.

other changes are very questionable... for example

pr_info("Added device: %s\n", zram->disk->disk_name);
becomes
dev_info(disk_to_dev(zram->disk), "Added device: %s\n", zram->disk->disk_name);

why? there is no reason to do this!

and messages are converted in a bit random manner, shall I say. so now we have a mix of dev_* and pr_* errors. to convert them all to dev_* (which is not possible in all the cases) we would need to change some function prototypes and start passing zram pointer, or disk...

example:

static struct zram_meta *zram_meta_alloc(int device_id, u64 disksize)

{
        size_t num_pages;
        char pool_name[8];
        struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL);

        if (!meta)
                return NULL;

        num_pages = disksize >> PAGE_SHIFT;
        meta->table = vzalloc(num_pages * sizeof(*meta->table));
        if (!meta->table) {
                pr_err("Error allocating zram address table\n");
                goto out_error;
        }

        snprintf(pool_name, sizeof(pool_name), "zram%d", device_id);
        meta->mem_pool = zs_create_pool(pool_name, GFP_NOIO | __GFP_HIGHMEM);
        if (!meta->mem_pool) {
                pr_err("Error creating memory pool\n");
                goto out_error;
        }

...

so I see a little value. really. too much things to change.

-ss

Joe Perches

2015-08-07 07:12 UTC #13 lore raw

Quoted 8 lines · Sergey Senozhatsky

On (08/07/15 15:37), Sergey Senozhatsky wrote:
[..]
where we had clean and nice
pr_err("Decompression failed!...
pr_info("Unable to allocate temp memory\n"...
etc...

Quoted 5 lines

now we have monsters
dev_err(disk_to_dev(zram->disk), "Decompression failed!... dev_info(disk_to_dev(zram->disk), "Unable to allocate temp memory\n"... etc.

[]

Quoted 6 lines

other changes are very questionable... for example
pr_info("Added device: %s\n", zram->disk->disk_name);
becomes
dev_info(disk_to_dev(zram->disk), "Added device: %s\n", zram->disk->disk_name);
why? there is no reason to do this!

This seems a reasonable complaint.

One option is to add some macros like

#define zram_err(zram, fmt, ...)				\
	dev_err(disk_to_dev((zram)->disk), fmt, ##__VA_ARGS__)

But the overall utility of the proposed changes is moderately low to non-existent.

Sergey Senozhatsky

2015-08-07 07:25 UTC #14 lore raw

Quoted 24 lines · Joe Perches

Quoted 8 lines · Sergey Senozhatsky
On (08/07/15 15:37), Sergey Senozhatsky wrote:
[..]
where we had clean and nice
pr_err("Decompression failed!...
pr_info("Unable to allocate temp memory\n"...
etc...
Quoted 5 lines
now we have monsters
dev_err(disk_to_dev(zram->disk), "Decompression failed!... dev_info(disk_to_dev(zram->disk), "Unable to allocate temp memory\n"... etc.
[]
Quoted 6 lines
other changes are very questionable... for example
pr_info("Added device: %s\n", zram->disk->disk_name);
becomes
dev_info(disk_to_dev(zram->disk), "Added device: %s\n", zram->disk->disk_name);
why? there is no reason to do this!
This seems a reasonable complaint.

Quoted 7 lines

One option is to add some macros like
#define zram_err(zram, fmt, ...)				\
	dev_err(disk_to_dev((zram)->disk), fmt, ##__VA_ARGS__)
But the overall utility of the proposed changes is moderately low to non-existent.

yes, sure. but we still need to change several internal functions to start accepting struct zram pointer just to be able to show extra prefix in error messages (if we want the messages to be more or less consistent).

-ss

Minchan Kim

maintainer

2015-08-07 14:59 UTC #15 lore raw

In reply to Sergey Senozhatsky

Quoted 34 lines · Sergey Senozhatsky

Hello Minchan,
On (08/07/15 15:05), Minchan Kim wrote:
[..]
Quoted 23 lines
Quoted 23 lines
I'd prefer to leave the messages the way they are. Changing anything visible to user space (api, eror codes, error messages, etc.) is a very risky business. You change the format of error messages and it smells like a big NO-NO.
'zram: Cannot initialise lzo compressing backend' --> 'block zram0: Cannot initialise lzo compressing backend'
And there are even more dramatic changes:
"Cannot change max compression streams\n" --> "Cannot change max compression streams to %d\n"
"Can't change algorithm for initialized device\n" --> "Can't change algorithm to %s for initialized device\n"
People already can have scripts doing grep "zram:" on dmesg or whatever. We cannot change this anymore.
This potentially breaks things in user space. So, I NACK the change set. Thanks.
Quoted 6 lines
Quoted 1 line
Minchan, any opinion?
Note: I didn't read this patchset in detail so I might be wrong.
When I read description, I couldn't see what's the benefit. Please write it out.

Quoted 8 lines

we now have errors like
'zram: Cannot initialise lzo compressing backend'
and they will transform into
'block zram0: Cannot initialise lzo compressing backend'
note the prefix 'zram:' became 'block zram0:'

It would be better because it's more clear if we can make several blocks.

Quoted 7 lines

and there are two patches (well, at least I quickly spotted only those) that change messages' text
From: "Cannot change max compression streams\n"
To: "Cannot change max compression streams to %d\n"

In this context, we don't need to say about max_comp_streams value. It's just fail regardless of the number of max stream you passed. Instead, it would be better to have prefix like above(ie, block zram0) for clarification/consistency.

Quoted 5 lines

where %d is a supplied max_comp_stream value
From: "Can't change algorithm for initialized device\n"
To: "Can't change algorithm to %s for initialized device\n"

It's okay but hope to have prefix "block zram0"

Quoted 5 lines

where %s is a supplied compression algorithm name.
as far as I can tell.

Thanks for the explain. Sergey!

I feel we should investigate all of error/log info places to make them clear/consistent if we decide to go with this.

Thanks.

Signature · 2 lines

Kind regards,
Minchan Kim

Sergey Senozhatsky

2015-08-10 01:24 UTC #16 lore raw

On (08/07/15 23:58), Minchan Kim wrote:
[..]

Quoted 4 lines

Quoted 1 line
note the prefix 'zram:' became 'block zram0:'
It would be better because it's more clear if we can make several blocks.

in that case I'd rather prefer to add zram%d to some of the existing messages, than mix pr_* with dev_*. besides, 'block zram0:' is a bit too long, a short 'zram0:' looks better.

well... we already return -errno from every path that does interact with user space, which is (I think) good enough; adding device_ids to those 'error return paths' may be OK.

for example. suppose zram is a swap device and we have !handle case

        handle = zs_malloc(meta->mem_pool, clen);
        if (!handle) {
                pr_info("Error allocating memory for compressed page: %u, size=%zu\n",
                        index, clen);
                ret = -ENOMEM;
                goto out;
        }

if zs_malloc() failed, then the system is in such big troubles, that device_id in the message is the last thing anyone will look at.

so I'm skeptical. the benefit is sort of minimal.

[..]

Quoted 7 lines

Quoted 5 lines
where %d is a supplied max_comp_stream value
From: "Can't change algorithm for initialized device\n"
To: "Can't change algorithm to %s for initialized device\n"
It's okay but hope to have prefix "block zram0"

it's not ok. I haven't tested it, but I think that the error message is broken. the supplied string most likely will contain trailing \n (I think people usually use echo, not echo -n), so it will be:
Can't change algorithm to XXX
for initialized device

... haven't tested.

BUT why would compression algorithm name even matter here?? the error is 'you attempted to configure _an already configured device_', not 'you attempted to configure an already configured device _with XXX_'. that XXX part is not relevant here, 'already configured device' is.

-ss