Messages in this thread | | | Date | Tue, 8 Sep 2015 10:14:43 +0900 | From | Minchan Kim <> | Subject | Re: [PATCH] zram: don't copy invalid compression algorithms |
| |
Hey Sergey,
On Tue, Sep 08, 2015 at 08:56:35AM +0900, Sergey Senozhatsky wrote: > On (09/07/15 21:48), Luis Henriques wrote: > > Validate the new compression algorithm before copying it into the zram > > 'compressor' field, keeping the old one if it's invalid. > > > > 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?
Thanks. > > > -ss > > > The error path code is also slightly refactored. > > > > Signed-off-by: Luis Henriques <luis.henriques@canonical.com> > > --- > > drivers/block/zram/zram_drv.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > > index 9c01f5bfa33f..33551ec9e7f5 100644 > > --- a/drivers/block/zram/zram_drv.c > > +++ b/drivers/block/zram/zram_drv.c > > @@ -367,10 +367,15 @@ 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"); > > - return -EBUSY; > > + len = -EBUSY; > > + goto out; > > + } > > + if (!zcomp_available_algorithm(buf)) { > > + len = -EINVAL; > > + goto out; > > } > > + > > strlcpy(zram->compressor, buf, sizeof(zram->compressor)); > > > > /* ignore trailing newline */ > > @@ -378,9 +383,7 @@ 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; > > - > > +out: > > up_write(&zram->init_lock); > > return len; > > } > >
| |