Messages in this thread | | | Date | Tue, 8 Sep 2015 10:33:38 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCH] zram: don't copy invalid compression algorithms |
| |
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
| |