From: David Rientjes <rientjes@google.com>
To: Jeff Liu <jeff.liu@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Christoph Lameter <cl@gentwo.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
Pekka Enberg <penberg@kernel.org>,
akpm@linuxfoundation.org
Subject: Re: [PATCH RESEND] slub: return correct error on slab_sysfs_init
Date: Fri, 20 Jun 2014 15:30:46 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.02.1406201526190.16090@chino.kir.corp.google.com> (raw)
In-Reply-To: <53A43C54.3090402@oracle.com>
On Fri, 20 Jun 2014, Jeff Liu wrote:
> At that time, I thought it would be ENOMEM because I was review another patch
> for adding sysfs support to XFS where we return ENOMEM in this case:
> http://www.spinics.net/lists/xfs/msg28343.html
>
> This drives to me to think why it should be ENOMEM rather than ERR_PTR since
> it seems most likely kset_create_and_add() would fails due to other reasons.
> Hence I looked through kernel sources and figured out most subsystems are return
> ENOMEM, maybe those subsystems are refers to the kset example code at:
> samples/kobject/kset-example.c
>
If you're going to ignore other emails in this thread, then you're not
going to make a very strong argument.
kset_create_and_add() can return NULL for reasons OTHER than just ENOMEM.
It can also be returned for EEXIST because something registered the
kobject with the same name. During init, which is what you're modifying
here, the liklihood is higher that it will return EEXIST rather than
ENOMEM otherwise there are much bigger problems than return value.
> So my original motivation is just to make the slub sysfs init error handling in
> accordance to other subsystems(nitpick) and it does not affect the kernel behaviour.
>
Why should slub match the other incorrect behavior?
What you're never addressing is WHY you are even making this change or
even care about the return value. Show userspace breakage that depends on
this.
> Combine with Greg's comments, as such, maybe the changelog would looks like
> the following?
>
> GregKH: the only reason for failure would be out of memory on kset_create_and_add().
> return -ENOMEM than -ENOSYS if the call is failed which is consistent with other
> subsystems in this situation.
>
Bullshit. Read the above.
If you want to return PTR_ERR() when this fails and fixup all the callers,
then propose that patch. Until then, it's a pretty simple rule: if you
don't have an errno, don't assume the reason for failure.
--
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:[~2014-06-20 22:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-18 1:29 Jeff Liu
2014-06-18 20:16 ` David Rientjes
2014-06-19 14:39 ` Christoph Lameter
2014-06-19 20:32 ` Andrew Morton
2014-06-19 21:34 ` David Rientjes
2014-06-20 13:51 ` Jeff Liu
2014-06-20 22:30 ` David Rientjes [this message]
2014-06-21 8:49 ` Jeff Liu
2014-06-23 13:59 ` Christoph Lameter
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=alpine.DEB.2.02.1406201526190.16090@chino.kir.corp.google.com \
--to=rientjes@google.com \
--cc=akpm@linux-foundation.org \
--cc=akpm@linuxfoundation.org \
--cc=cl@gentwo.org \
--cc=jeff.liu@oracle.com \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
/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