lkml.org 
[lkml]   [2019]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] livepatch: core: Return ENOTSUPP instead of ENOSYS
On 2019-01-30 21:41, Petr Mladek wrote:
> On Tue 2019-01-29 10:50:54, Josh Poimboeuf wrote:
>> On Mon, Jan 28, 2019 at 02:49:43PM -0500, Joe Lawrence wrote:
>> > On Sun, Jan 27, 2019 at 04:26:30AM +0900, Alice Ferrazzi wrote:
>> > > This patch fixes a checkpatch warning:
>> > > WARNING: ENOSYS means 'invalid syscall nr' and nothing else
>> > >
>> > > Signed-off-by: Alice Ferrazzi <alice.ferrazzi@miraclelinux.com>
>> > > ---
>> > > kernel/livepatch/core.c | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> > > index 5b77a7314e01..eea6b94fef89 100644
>> > > --- a/kernel/livepatch/core.c
>> > > +++ b/kernel/livepatch/core.c
>> > > @@ -897,7 +897,7 @@ int klp_register_patch(struct klp_patch *patch)
>> > >
>> > > if (!klp_have_reliable_stack()) {
>> > > pr_err("This architecture doesn't have support for the livepatch consistency model.\n");
>> > > - return -ENOSYS;
>> > > + return -ENOTSUPP;
>> > > }
>> > >
>> > > return klp_init_patch(patch);
>> > > --
>> > > 2.19.2
>> > >
>> >
>> > Hi Alice,
>> >
>> > Patches should be based off the upstream livepatching tree, found here:
>> >
>> > git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching.git
>> >
>> > and in this case, the for-next branch, which holds patches that have
>> > already been queued up for the next release. This one:
>> >
>> > 958ef1e39d24 ("livepatch: Simplify API by removing registration step")
>> >
>> > has moved the code in question from klp_register_patch() to
>> > klp_enable_patch().
>> >
>> >
>> > As far as the change itself, I don't have strong opinion about it
>> > either way.
>> >
>> > On the one hand, there is the checkpatch warning and -ENOTSUPP reads
>> > more intuitively than -ENOSYS.
>> >
>> > However, the current pattern seems to be more prevelent in the kernel.
>> > I wonder if the checkpatch warning would be better specified for return
>> > values that are actually passed back to userspace.
>> >
>> > Also, klp_register_patch(), now klp_enable_patch(), is exported for
>> > module use, though I don't believe anyone (samples / tests / kpatch /
>> > kgraft?) is inspecting which error value is returned.
>> >
>> > I would defer to whichever convention the maintainers prefer here.
>>
>> Based on the commit description from 91c9afaf97ee ("checkpatch.pl: new
>> instances of ENOSYS are errors"), it sounds like there was a decision
>> at
>> Kernel Summit to limit ENOSYS to mean "bad syscall" and nothing else.
>
> Hmm, the error code is passed to the syscall, for example:
>
> + SYSCALL_DEFINE3(init_module
> + load_module()
> + do_init_module()
> + do_one_initcall(mod->init);
>
> I am not sure if we are allowed to return -ENOTSUPP (-524).
> It is defined in the internal include/linux/errno.h. There
> is the following commnent:
>
> /*
> * These should never be seen by user programs...
>
>
>
> I tried to find a better alternative and found:
>
> #define EOPNOTSUPP 95 /* Operation not supported on transport endpoint
> */
>
>
> There is the following note in man errno:
>
> ENOTSUP Operation not supported (POSIX.1)
>
> EOPNOTSUPP Operation not supported on socket (POSIX.1)
> (ENOTSUP and EOPNOTSUPP have the same value
> on Linux, but according to POSIX.1 these error
> values should be distinct.)
>
> And it looks that -EOPNOTSUPP is used widely in many subsystes (not
> only network).
>
> Best Regards,
> Petr


EOPNOTSUPP works also for me.
looks better adopted than ENOTSUP.

I will send a new patch based off the upstream from
git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching.git
as suggested by Joe

Thanks,
Alice


--
======================================
Alice Ferrazzi
alicef@alicef.me
PGP: 2E4E 0856 461C 0585 1336 F496 5621 A6B2 8638 781A
======================================

\
 
 \ /
  Last update: 2019-01-30 14:01    [W:0.060 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site