From: Jeff Layton <jlayton@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk,
lhenriques@suse.com, cmaiolino@redhat.com,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] mm: check for sleepable context in kvfree
Date: Tue, 23 Jul 2019 14:05:11 -0400 [thread overview]
Message-ID: <f43c131d9b635994aafed15cb72308b32d2eef67.camel@kernel.org> (raw)
In-Reply-To: <20190723175543.GL363@bombadil.infradead.org>
On Tue, 2019-07-23 at 10:55 -0700, Matthew Wilcox wrote:
> On Tue, Jul 23, 2019 at 01:52:36PM -0400, Jeff Layton wrote:
> > On Tue, 2019-07-23 at 09:12 -0400, Jeff Layton wrote:
> > > A lot of callers of kvfree only go down the vfree path under very rare
> > > circumstances, and so may never end up hitting the might_sleep_if in it.
> > > Ensure that when kvfree is called, that it is operating in a context
> > > where it is allowed to sleep.
> > >
> > > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > > Cc: Luis Henriques <lhenriques@suse.com>
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > > mm/util.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> >
> > FWIW, I started looking at this after Luis sent me some ceph patches
> > that fixed a few of these problems. I have not done extensive testing
> > with this patch, so maybe consider this an RFC for now.
> >
> > HCH points out that xfs uses kvfree as a generic "free this no matter
> > what it is" sort of wrapper and expects the callers to work out whether
> > they might be freeing a vmalloc'ed address. If that sort of usage turns
> > out to be prevalent, then we may need another approach to clean this up.
>
> I think it's a bit of a landmine, to be honest. How about we have kvfree()
> call vfree_atomic() instead?
Not a bad idea, though it means more overhead for the vfree case.
Since we're spitballing here...could we have kvfree figure out whether
it's running in a context where it would need to queue it instead and
only do it in that case?
We currently have to figure that out for the might_sleep_if anyway. We
could just have it DTRT instead of printk'ing and dumping the stack in
that case.
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2019-07-23 18:05 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-23 13:12 [PATCH] mm: check for sleepable context in kvfree Jeff Layton
2019-07-23 17:52 ` Jeff Layton
2019-07-23 17:55 ` Matthew Wilcox
2019-07-23 18:05 ` Jeff Layton [this message]
2019-07-23 18:11 ` Matthew Wilcox
2019-07-23 18:19 ` Jeff Layton
2019-07-23 18:29 ` Matthew Wilcox
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=f43c131d9b635994aafed15cb72308b32d2eef67.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=cmaiolino@redhat.com \
--cc=hch@lst.de \
--cc=lhenriques@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).