Messages in this thread Patch in this message | | | Date | Wed, 9 Sep 2015 09:45:18 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCH v2] zram: introduce comp algorithm fallback functionality |
| |
On (09/08/15 19:42), Luis Henriques wrote: > > Note that previously this operation (i.e. setting an invalid algorithm) > would result in no algorithm being selected, which means that this > represents a small change in the default behaviour. >
previously it would result in guaranteed to fail echo xxx > /sys/block/zram<id>/disksize and thus in no device, not just "in no algorithm being selected".
comp_algorithm historically wasn't returning anything back to user space and it always copied in its input (what later would have been used as a compression algorithm name). yes, "wasn't returning anything back" is not entirely correct, there was (and still is) a chance to receive -EBUSY, but that was (and still is) is absolutely equivalent to /sys/block/zram<id>/initstate != 0
(well, I never had a clear understanding of why comp_algorithm and other attrs return -EBUSY instead of simply silently ignoring the input; is it really an error? there is a initstate attr for that, but I never had enough reasons to change it either). anyway, believe me or not, that's what my toy scripts were doing for years (and hopefully it's not because of the fact that I'm completely unaware of the trivial basics of software development, as we found out yesterday, but because zram ABI was letting me to do so).
# head -17 create-zram --- #!/bin/sh
rmmod zram modprobe zram
if [ -e /sys/block/zram0/initstate ]; then initdone=`cat /sys/block/zram0/initstate` if [ $initdone = 1 ]; then echo "init done" exit 1 fi fi
echo 8 > /sys/block/zram0/max_comp_streams
echo $1 > /sys/block/zram0/comp_algorithm cat /sys/block/zram0/comp_algorithm
---
So, "./create-zram LLZZOO 2G" will have different result now.
Other than that
Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
-ss
> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > Signed-off-by: Luis Henriques <luis.henriques@canonical.com> > --- > Changes since v1: > * Moved the algorithm check further up, before mutex lock > * Dropped error path refactoring > * Updated commit text to explicitly refer to the ABI change > (changes suggested by Sergey and Minchan) > > drivers/block/zram/zram_drv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 9c01f5bfa33f..1caa8f793e51 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -365,6 +365,9 @@ static ssize_t comp_algorithm_store(struct device *dev, > struct zram *zram = dev_to_zram(dev); > size_t sz; > > + if (!zcomp_available_algorithm(buf)) > + return -EINVAL; > + > down_write(&zram->init_lock); > if (init_done(zram)) { > up_write(&zram->init_lock); > @@ -378,9 +381,6 @@ static ssize_t comp_algorithm_store(struct device *dev, > if (sz > 0 && zram->compressor[sz - 1] == '\n') > zram->compressor[sz - 1] = 0x00; > > - if (!zcomp_available_algorithm(zram->compressor)) > - len = -EINVAL; > - > up_write(&zram->init_lock); > return len; > } >
| |