Re: [PATCH] zram: don't copy invalid compression algorithms

Minchan Kim
2015-09-08T08:15:35Z

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