public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov@virtuozzo.com>
To: Dmitry Safonov <dsafonov@virtuozzo.com>
Cc: <akpm@linux-foundation.org>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>, <0x7f454c46@gmail.com>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH] mm: slab: free kmem_cache_node after destroy sysfs file
Date: Mon, 8 Feb 2016 12:00:34 +0300	[thread overview]
Message-ID: <20160208090034.GA30053@esperanza> (raw)
In-Reply-To: <56B85663.9030406@virtuozzo.com>

On Mon, Feb 08, 2016 at 11:48:35AM +0300, Dmitry Safonov wrote:
...
> >>  /*
> >>   * Attempt to free all partial slabs on a node.
> >>- * This is called from kmem_cache_close(). We must be the last thread
> >>+ * This is called from __kmem_cache_shutdown(). We must be the last thread
> >>   * using the cache and therefore we do not need to lock anymore.
> >Well, that's not true as we've found out - sysfs might still access the
> >cache in parallel. And alloc_calls_show -> list_locations does walk over
> >the kmem_cache_node->partial list, which we prune on shutdown.
> >
> >I guess we should reintroduce locking for free_partial() in the scope of
> >this patch, partially reverting 69cb8e6b7c298.
> I think, we can omit locking for !SLAB_SUPPORTS_SYSFS and reintroduce
> for sysfs case. Will do

I really don't think there's any point in cluttering the code with
ifdefs here - we'd better just enable locking in any case. It won't hurt
performance, because it's a very-very slow path anyway. Besides, SYSFS
is on by default on most builds.

FWIW SLAB does not omit locking on shutdown, although it doesn't support
sysfs.

Thanks,
Vladimir

      reply	other threads:[~2016-02-08  9:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-05 17:16 [PATCH] mm: slab: free kmem_cache_node after destroy sysfs file Dmitry Safonov
2016-02-07 19:10 ` Vladimir Davydov
2016-02-08  8:48   ` Dmitry Safonov
2016-02-08  9:00     ` Vladimir Davydov [this message]

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=20160208090034.GA30053@esperanza \
    --to=vdavydov@virtuozzo.com \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=dsafonov@virtuozzo.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.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