On (09/08/15 23:21), Minchan Kim wrote:
[..]
> > > If you ignore tailling space, your change would make a bug.
> > > If you fix it, it looks good to me.
> >
> > that's why find_backend() calls sysfs_streq(), which takes care of
> > a trailing new line. unless you're speaking about something else.
On Tue, Sep 08, 2015 at 10:44:01PM +0900, Sergey Senozhatsky wrote:
> dammit, not going to waste my time on this BS anymore.
>
>
>
> > > --- 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
> >
dammit, not going to waste my time on this BS anymore.
> > --- 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;
> >
> > +
On Tue, Sep 08, 2015 at 06:54:28PM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 17:16), Minchan Kim wrote:
> > we should help them to *correct* it rather than keeping such weired
> > thing.
>
> A simple quiz
>
> A)
> echo zzz > /sys/block/zram0/comp_algorithm > /dev/null
> echo 1G >
On (09/08/15 11:08), Luis Henriques wrote:
[..]
> >
> > /sys/block/sdb/queue/scheduler
>
> This one is actually a good example
>
... its behaviour depends on the way the kernel uses that queue.
# echo THIS_EXIST_ONLY_IN_MY_HEAD >
/sys/devices/virtual/block/zram0/queue/scheduler
# echo $?
0
#
things popped from my mind
> at the moment.
>
> /sys/block/sdb/queue/scheduler
This one is actually a good example, and its behaviour is exactly what I
expecting in zram.
But I believe To: Minchan Kim
Cc: Sergey Senozhatsky ,
linux-kernel@vger.kernel.org,
Sergey Senozhat
On (09/08/15 17:16), Minchan Kim wrote:
> we should help them to *correct* it rather than keeping such weired
> thing.
A simple quiz
A)
echo zzz > /sys/block/zram0/comp_algorithm > /dev/null
echo 1G > /sys/block/zram0/disksize
-bash: echo: write error: Invalid argument
echo $?
1
B)
echo zzz >
On Tue, Sep 08, 2015 at 02:04:42PM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 13:50), Minchan Kim wrote:
> [..]
> > And it's straightforward/consistent to change the thing's state
> > only if is successful.
> >
>
> what for? I provided several good reasons not to do this, because
Several
On Tue, Sep 08, 2015 at 02:04:42PM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 13:50), Minchan Kim wrote:
> [..]
> > And it's straightforward/consistent to change the thing's state
> > only if is successful.
> >
>
> what for? I provided several good reasons not to do this, because
Several
On (09/08/15 17:16), Minchan Kim wrote:
> we should help them to *correct* it rather than keeping such weired
> thing.
A simple quiz
A)
echo zzz > /sys/block/zram0/comp_algorithm > /dev/null
echo 1G > /sys/block/zram0/disksize
-bash: echo: write error: Invalid argument
echo $?
1
B)
echo zzz >
On (09/08/15 11:08), Luis Henriques wrote:
[..]
> >
> > /sys/block/sdb/queue/scheduler
>
> This one is actually a good example
>
... its behaviour depends on the way the kernel uses that queue.
# echo THIS_EXIST_ONLY_IN_MY_HEAD >
/sys/devices/virtual/block/zram0/queue/scheduler
# echo $?
0
#
things popped from my mind
> at the moment.
>
> /sys/block/sdb/queue/scheduler
This one is actually a good example, and its behaviour is exactly what I
expecting in zram.
But I believe To: Minchan Kim <minc...@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.w...@gm
dammit, not going to waste my time on this BS anymore.
> > --- 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;
> >
> > +
On Tue, Sep 08, 2015 at 06:54:28PM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 17:16), Minchan Kim wrote:
> > we should help them to *correct* it rather than keeping such weired
> > thing.
>
> A simple quiz
>
> A)
> echo zzz > /sys/block/zram0/comp_algorithm > /dev/null
> echo 1G >
On (09/08/15 23:21), Minchan Kim wrote:
[..]
> > > If you ignore tailling space, your change would make a bug.
> > > If you fix it, it looks good to me.
> >
> > that's why find_backend() calls sysfs_streq(), which takes care of
> > a trailing new line. unless you're speaking about something else.
On Tue, Sep 08, 2015 at 10:44:01PM +0900, Sergey Senozhatsky wrote:
> dammit, not going to waste my time on this BS anymore.
>
>
>
> > > --- 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
> >
On (09/08/15 13:50), Minchan Kim wrote:
> For exmaple, disksize, max_comp_streams are changed only if
> it is successful.
> If your logic were right approach, we should change
> max_comp_streams for *stupid* script although it doesn't check
define stupid.
is echo 210 >
On (09/08/15 13:50), Minchan Kim wrote:
[..]
> And it's straightforward/consistent to change the thing's state
> only if is successful.
>
what for? I provided several good reasons not to do this, because
it makes life easier for users. we added this check in Jun 25, 2015
while this functionality
On Tue, Sep 08, 2015 at 10:58:31AM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 10:33), Sergey Senozhatsky wrote:
> > > 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
On (09/08/15 10:33), Sergey Senozhatsky wrote:
> > 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
>
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
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
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()
On (09/08/15 13:50), Minchan Kim wrote:
> For exmaple, disksize, max_comp_streams are changed only if
> it is successful.
> If your logic were right approach, we should change
> max_comp_streams for *stupid* script although it doesn't check
define stupid.
is echo 210 >
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()
On (09/08/15 10:33), Sergey Senozhatsky wrote:
> > 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
>
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
On Tue, Sep 08, 2015 at 10:58:31AM +0900, Sergey Senozhatsky wrote:
> On (09/08/15 10:33), Sergey Senozhatsky wrote:
> > > 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
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
On (09/08/15 13:50), Minchan Kim wrote:
[..]
> And it's straightforward/consistent to change the thing's state
> only if is successful.
>
what for? I provided several good reasons not to do this, because
it makes life easier for users. we added this check in Jun 25, 2015
while this functionality
30 matches
Mail list logo