Re: [PATCH] zram: don't copy invalid compression algorithms

[Sergey Senozhatsky]

On (09/08/15 10:14), Minchan Kim wrote:
[..]
> > NACK.
> > 
> > This is intentional. We haven't returned 'invalid compression algorithm'
> > error from comp_algorithm_store() historically, so someone's script can
> > simply ignore it. However, the script will fail to init the device and
> > user will be able to figure out the root cause, because zram will report
> > to syslog an actually requested alg name.
> > 
> > Example
> > 
> > [ 1669.473296] zram: Cannot initialise llzo compressing backend
> 
> I don't understand your concern. To me, this patch makes sense to me.
> Could you explain your point clearly, again?

OK. suppose someone landed a typo in a 'zram device management' script

echo llzo > /sys/block/zram0/comp_algorithm
-bash: echo: write error: Invalid argument


but the script ignores 'echo: write error'.
Because we added compression algorithm name check recently.

then the script does

echo 200M > /sys/block/zram0/disksize
-bash: echo: write error: Invalid argument


doing a simple dmesg reveals the problem

[ 7076.657184] zram: Cannot initialise llzo compressing backend

note that zram provides 'llzo' here, which is convenient.


With this change the semantics is changing and zram now swallows
(hides) the user space error.

echo llzo > /sys/block/zram0/comp_algorithm
-bash: echo: write error: Invalid argument

echo 200M > /sys/block/zram0/disksize

instead of using a requested 'llzo' zram for some reason
switched to 'lzo'.


I don't like this behaviour change. User requested to change
the 'default' value, and that new value didn't work out. No
reason to hide it.

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/