From: Marcin Jabrzyk <m.jabrzyk@samsung.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: minchan@kernel.org, ngupta@vflare.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
kyungmin.park@samsung.com
Subject: Re: [PATCH] zram: check compressor name before setting it
Date: Fri, 22 May 2015 15:26:20 +0200 [thread overview]
Message-ID: <555F2E7C.4090707@samsung.com> (raw)
In-Reply-To: <20150522124411.GA3793@swordfish>
On 22/05/15 14:44, Sergey Senozhatsky wrote:
> On (05/22/15 11:12), Marcin Jabrzyk wrote:
>>>
>>> no.
>>>
>>> zram already complains about failed comp backend creation.
>>> it's in dmesg (or syslog, etc.):
>>>
>>> "zram: Cannot initialise %s compressing backend"
>>>
>> OK, now I see that. Sorry for the noise.
>>
>>> second, there is not much value in exposing zcomp internals,
>>> especially when the result is just another line in dmesg output.
>>
>> From the other hand, the only valid values that can be written are
>> in 'comp_algorithm'.
>> So when writing other one, returning -EINVAL seems to be reasonable.
>> The user would get immediately information that he can't do that,
>> now the information can be very deferred in time.
>
> it's not.
> the error message appears in syslog right before we return -EINVAL
> back to user.
Yes I've read up the code more detailed and I saw that error message
just before returning to user with error value.
But this happens when 'disksize' is wirtten, not when 'comp_algorithm'.
I understood, the error message in dmesg is clear there is no such
algorithm.
But this is not an immediate error, when setting the 'comp_algorithm',
where we already know that it's wrong, not existing etc.
Anything after that moment would be wrong and would not work at all.
From what I saw 'comp_algorithm_store' is the only *_store in zram that
believes user that he writes proper value and just makes strlcpy.
So what I've ing mind is to provide direct feedback, you have
written wrong name of compressor, you got -EINVAL, please write
correct value. This would be very useful when scripting.
Sorry for being so confusing.
Best regards,
Marcin Jabrzyk
>
> -ss
>
>> I'm not for exposing more internals, but getting -EINVAL would be nice I
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-05-22 13:26 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-22 8:31 Marcin Jabrzyk
2015-05-22 8:56 ` Sergey Senozhatsky
2015-05-22 9:12 ` Marcin Jabrzyk
2015-05-22 12:44 ` Sergey Senozhatsky
2015-05-22 13:14 ` Minchan Kim
2015-05-22 13:34 ` Marcin Jabrzyk
2015-05-25 4:03 ` Sergey Senozhatsky
2015-05-25 14:16 ` Minchan Kim
2015-05-22 13:26 ` Marcin Jabrzyk [this message]
2015-05-25 6:18 ` Sergey Senozhatsky
2015-05-25 6:23 ` Sergey Senozhatsky
2015-05-25 7:15 ` Marcin Jabrzyk
2015-05-25 7:34 ` Sergey Senozhatsky
2015-05-25 8:05 ` Marcin Jabrzyk
2015-05-25 14:21 ` Minchan Kim
2015-05-26 0:09 ` 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=555F2E7C.4090707@samsung.com \
--to=m.jabrzyk@samsung.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=ngupta@vflare.org \
--cc=sergey.senozhatsky.work@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