Re: [PATCH 0/3] zram: Replace pr_* with dev_*

Sergey Senozhatsky
2015-08-10T01:24:54Z

On (08/07/15 23:58), Minchan Kim wrote:
[..]
> > 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.

[..]
> >
> > 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