linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Luis Henriques <luis.henriques@canonical.com>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCH] zram: don't copy invalid compression algorithms
Date: Tue, 8 Sep 2015 17:16:10 +0900	[thread overview]
Message-ID: <20150908081610.GA8633@bbox> (raw)
In-Reply-To: <20150908050442.GA609@swordfish>

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 good reasons?

I just heard you claim to take care of scripts which don't check
function's success at the moment function is called but check it
later via reading the knob later.
If we changes it, it breaks such scripts.
So, with your claim, there are two assumption.

1. script doesn't check return val at the moment function completes
2. Instead, script checks it later via reading the knob again.
So, conclusion is we should keep wrong input in kernel side for them.

It seems you insist on "we should keep wrong input from the userspace
in the kernel to show it if user *might* ask for his debug later"
What makes you think like above?

I think such assumption is really from your brain, not real usecases.
Ok, I admit i'm not a god but if there is such thing in real practice,
we should help them to *correct* it rather than keeping such weired
thing. From the beginning, they should check his action's result with
return value, not dmesg, not reading the knob later, again.

> it makes life easier for users. we added this check in Jun 25, 2015

No, it could make more bad scripts which not checks the result
of the action but rely on current awkward zram's interface.
Consider other knobs in the kernel. A few things popped from my mind
at the moment.

/sys/block/sdb/queue/scheduler
/sys/kernel/mm/transparent_hugepage/enabled
/sys/kernel/debug/tracing/current_tracer
/sys/devices/system/clocksource/clocksource/clocksource0/current_clocksource

They are not showing wrong input user have passed although it was failed.
Could you say a example of kernel interface did intentionally like you said?

> while this functionality and scripts have been around for years, and
> apparently now it's users' problem and they must go and do something.

I believe anyone shouldn't rely on it. But who knows?
However, I want to make it sane(ie, only change compressor name
if the action is successful).
Please, let's discuss how we do it rather than whether it's useful
or not.

> 
> 
> seriously, what improvement this change brings in the first place?
> what does it make better and for whom?

As I mentioned, it makes zram's ABI consistent with others
in kernel space so it makes user feel zram is straight-forward
and sane like others.


> 
> 	-ss

  reply	other threads:[~2015-09-08  8:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-07 20:48 [PATCH] zram: don't copy invalid compression algorithms Luis Henriques
2015-09-07 23:56 ` Sergey Senozhatsky
2015-09-08  1:14   ` Minchan Kim
2015-09-08  1:33     ` Sergey Senozhatsky
2015-09-08  1:58       ` Sergey Senozhatsky
2015-09-08  4:50         ` Minchan Kim
2015-09-08  5:04           ` Sergey Senozhatsky
2015-09-08  8:16             ` Minchan Kim [this message]
2015-09-08  9:54               ` Sergey Senozhatsky
2015-09-08 13:33                 ` Minchan Kim
2015-09-08 13:44                   ` Sergey Senozhatsky
2015-09-08 14:21                     ` Minchan Kim
2015-09-08 15:35                       ` Sergey Senozhatsky
2015-09-08 10:08               ` Luis Henriques
2015-09-08 12:20                 ` Sergey Senozhatsky
2015-09-08  5:15           ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150908081610.GA8633@bbox \
    --to=minchan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luis.henriques@canonical.com \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).