All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>,
	Roman Gushchin <guroan@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com, Johannes Weiner <hannes@cmpxchg.org>,
	Vlastimil Babka <vbabka@suse.cz>, Roman Gushchin <guro@fb.com>,
	Christoph Hellwig <hch@lst.de>,
	Joel Fernandes <joelaf@google.com>
Subject: Re: [PATCH v4 1/2] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
Date: Fri, 19 Apr 2019 20:08:11 +0100	[thread overview]
Message-ID: <20190419190811.GF2217@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190418152431.c583ef892a8028c662db3e6a@linux-foundation.org>

On Thu, Apr 18, 2019 at 03:24:31PM -0700, Andrew Morton wrote:
> On Thu, 18 Apr 2019 04:18:34 -0700 Matthew Wilcox <willy@infradead.org> wrote:
> 
> > On Wed, Apr 17, 2019 at 02:58:27PM -0700, Andrew Morton wrote:
> > > On Wed, 17 Apr 2019 12:40:01 -0700 Roman Gushchin <guroan@gmail.com> wrote:
> > > > +static struct vm_struct *__remove_vm_area(struct vmap_area *va)
> > > > +{
> > > > +	struct vm_struct *vm = va->vm;
> > > > +
> > > > +	might_sleep();
> > > 
> > > Where might __remove_vm_area() sleep?
> > > 
> > > >From a quick scan I'm only seeing vfree(), and that has the
> > > might_sleep_if(!in_interrupt()).
> > > 
> > > So perhaps we can remove this...
> > 
> > See commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as potentially sleeping")
> > 
> > It looks like the intent is to unconditionally check might_sleep() at
> > the entry points to the vmalloc code, rather than only catch them in
> > the occasional place where it happens to go wrong.
> 
> afaict, vfree() will only do a mutex_trylock() in
> try_purge_vmap_area_lazy().  So does vfree actually sleep in any
> situation?  Whether or not local interrupts are enabled?

IIRC, the original problem that used to prohibit vfree() in interrupts
was the use of spinlocks that were used in a lot of places by plain
spin_lock().  I'm not sure it could actually sleep in anything not
too ancient...


  parent reply	other threads:[~2019-04-19 19:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-17 19:40 [PATCH v4 0/2] vmalloc enhancements Roman Gushchin
2019-04-17 19:40 ` [PATCH v4 1/2] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
2019-04-17 21:58   ` Andrew Morton
2019-04-17 23:02     ` Roman Gushchin
2019-04-18 11:18     ` Matthew Wilcox
2019-04-18 22:24       ` Andrew Morton
2019-04-18 23:17         ` Eric Dumazet
2019-04-19 19:08         ` Al Viro [this message]
2019-04-17 19:40 ` [PATCH v4 2/2] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin

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=20190419190811.GF2217@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=guro@fb.com \
    --cc=guroan@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=hch@lst.de \
    --cc=joelaf@google.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=vbabka@suse.cz \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.