All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Tejun Heo <tj@kernel.org>
Cc: Vladimir Davydov <vdavydov@tarantool.org>,
	cl@linux.com, penberg@kernel.org, rientjes@google.com,
	akpm@linux-foundation.org, jsvana@fb.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	cgroups@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path
Date: Tue, 17 Jan 2017 09:07:54 +0900	[thread overview]
Message-ID: <20170117000754.GA25218@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <20170114151921.GA32693@mtj.duckdns.org>

On Sat, Jan 14, 2017 at 10:19:21AM -0500, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Sat, Jan 14, 2017 at 04:19:39PM +0300, Vladimir Davydov wrote:
> > On Sat, Jan 14, 2017 at 12:54:42AM -0500, Tejun Heo wrote:
> > > This patch updates the cache release path so that it simply uses
> > > call_rcu() instead of the synchronous rcu_barrier() + custom batching.
> > > This doesn't cost more while being logically simpler and way more
> > > scalable.
> > 
> > The point of rcu_barrier() is to wait until all rcu calls freeing slabs
> > from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free).
> > I'm not sure if call_rcu() guarantees that for all rcu implementations
> > too. If it did, why would we need rcu_barrier() at all?
> 
> Yeah, I had a similar question and scanned its users briefly.  Looks
> like it's used in combination with ctors so that its users can
> opportunistically dereference objects and e.g. check ids / state /
> whatever without worrying about the objects' lifetimes.

Hello, Tejun.

Long time no see! :)

IIUC, rcu_barrier() here prevents to destruct the kmem_cache until all
slab pages in it are freed. These slab pages are freed through call_rcu().

Your patch changes it to another call_rcu() and, I think, if sequence of
executing rcu callbacks is the same with sequence of adding rcu
callbacks, it would work. However, I'm not sure that it is
guaranteed by RCU API. Am I missing something?

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Tejun Heo <tj@kernel.org>
Cc: Vladimir Davydov <vdavydov@tarantool.org>,
	cl@linux.com, penberg@kernel.org, rientjes@google.com,
	akpm@linux-foundation.org, jsvana@fb.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	cgroups@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path
Date: Tue, 17 Jan 2017 09:07:54 +0900	[thread overview]
Message-ID: <20170117000754.GA25218@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <20170114151921.GA32693@mtj.duckdns.org>

On Sat, Jan 14, 2017 at 10:19:21AM -0500, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Sat, Jan 14, 2017 at 04:19:39PM +0300, Vladimir Davydov wrote:
> > On Sat, Jan 14, 2017 at 12:54:42AM -0500, Tejun Heo wrote:
> > > This patch updates the cache release path so that it simply uses
> > > call_rcu() instead of the synchronous rcu_barrier() + custom batching.
> > > This doesn't cost more while being logically simpler and way more
> > > scalable.
> > 
> > The point of rcu_barrier() is to wait until all rcu calls freeing slabs
> > from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free).
> > I'm not sure if call_rcu() guarantees that for all rcu implementations
> > too. If it did, why would we need rcu_barrier() at all?
> 
> Yeah, I had a similar question and scanned its users briefly.  Looks
> like it's used in combination with ctors so that its users can
> opportunistically dereference objects and e.g. check ids / state /
> whatever without worrying about the objects' lifetimes.

Hello, Tejun.

Long time no see! :)

IIUC, rcu_barrier() here prevents to destruct the kmem_cache until all
slab pages in it are freed. These slab pages are freed through call_rcu().

Your patch changes it to another call_rcu() and, I think, if sequence of
executing rcu callbacks is the same with sequence of adding rcu
callbacks, it would work. However, I'm not sure that it is
guaranteed by RCU API. Am I missing something?

Thanks.

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim-Hm3cg6mZ9cc@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vladimir Davydov
	<vdavydov-mJ5StSI9eqKXDw4h08c5KA@public.gmane.org>,
	cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org,
	penberg-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	jsvana-b10kYP2dOMg@public.gmane.org,
	hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-team-b10kYP2dOMg@public.gmane.org
Subject: Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path
Date: Tue, 17 Jan 2017 09:07:54 +0900	[thread overview]
Message-ID: <20170117000754.GA25218@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <20170114151921.GA32693-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>

On Sat, Jan 14, 2017 at 10:19:21AM -0500, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Sat, Jan 14, 2017 at 04:19:39PM +0300, Vladimir Davydov wrote:
> > On Sat, Jan 14, 2017 at 12:54:42AM -0500, Tejun Heo wrote:
> > > This patch updates the cache release path so that it simply uses
> > > call_rcu() instead of the synchronous rcu_barrier() + custom batching.
> > > This doesn't cost more while being logically simpler and way more
> > > scalable.
> > 
> > The point of rcu_barrier() is to wait until all rcu calls freeing slabs
> > from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free).
> > I'm not sure if call_rcu() guarantees that for all rcu implementations
> > too. If it did, why would we need rcu_barrier() at all?
> 
> Yeah, I had a similar question and scanned its users briefly.  Looks
> like it's used in combination with ctors so that its users can
> opportunistically dereference objects and e.g. check ids / state /
> whatever without worrying about the objects' lifetimes.

Hello, Tejun.

Long time no see! :)

IIUC, rcu_barrier() here prevents to destruct the kmem_cache until all
slab pages in it are freed. These slab pages are freed through call_rcu().

Your patch changes it to another call_rcu() and, I think, if sequence of
executing rcu callbacks is the same with sequence of adding rcu
callbacks, it would work. However, I'm not sure that it is
guaranteed by RCU API. Am I missing something?

Thanks.

  reply	other threads:[~2017-01-17  0:02 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-14  5:54 [PATCHSET] slab: make memcg slab destruction scalable Tejun Heo
2017-01-14  5:54 ` Tejun Heo
2017-01-14  5:54 ` Tejun Heo
2017-01-14  5:54 ` [PATCH 1/9] Revert "slub: move synchronize_sched out of slab_mutex on shrink" Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14  5:54 ` [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:19   ` Vladimir Davydov
2017-01-14 13:19     ` Vladimir Davydov
2017-01-14 15:19     ` Tejun Heo
2017-01-14 15:19       ` Tejun Heo
2017-01-17  0:07       ` Joonsoo Kim [this message]
2017-01-17  0:07         ` Joonsoo Kim
2017-01-17  0:07         ` Joonsoo Kim
2017-01-17 16:37         ` Tejun Heo
2017-01-17 16:37           ` Tejun Heo
2017-01-17 17:02           ` Tejun Heo
2017-01-17 17:02             ` Tejun Heo
2017-01-14  5:54 ` [PATCH 3/9] slab: simplify shutdown_memcg_caches() Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:27   ` Vladimir Davydov
2017-01-14 13:27     ` Vladimir Davydov
2017-01-14 15:38     ` Tejun Heo
2017-01-14 15:38       ` Tejun Heo
2017-01-14 15:53       ` Tejun Heo
2017-01-14 15:53         ` Tejun Heo
2017-01-14  5:54 ` [PATCH 4/9] slab: reorganize memcg_cache_params Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:30   ` Vladimir Davydov
2017-01-14 13:30     ` Vladimir Davydov
2017-01-14  5:54 ` [PATCH 5/9] slab: link memcg kmem_caches on their associated memory cgroup Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:33   ` Vladimir Davydov
2017-01-14 13:33     ` Vladimir Davydov
2017-01-14  5:54 ` [PATCH 6/9] slab: don't put memcg caches on slab_caches list Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:39   ` Vladimir Davydov
2017-01-14 13:39     ` Vladimir Davydov
2017-01-14 15:39     ` Tejun Heo
2017-01-14 15:39       ` Tejun Heo
2017-01-14 15:39       ` Tejun Heo
2017-01-14  5:54 ` [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate() Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:42   ` Vladimir Davydov
2017-01-14 13:42     ` Vladimir Davydov
2017-01-14 15:39     ` Tejun Heo
2017-01-14 15:39       ` Tejun Heo
2017-01-14  5:54 ` [PATCH 8/9] slab: remove synchronous synchronize_sched() from memcg cache deactivation path Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 13:57   ` Vladimir Davydov
2017-01-14 13:57     ` Vladimir Davydov
2017-01-14 13:57     ` Vladimir Davydov
2017-01-14  5:54 ` [PATCH 9/9] slab: remove slub sysfs interface files early for empty memcg caches Tejun Heo
2017-01-14  5:54   ` Tejun Heo
2017-01-14 14:00   ` Vladimir Davydov
2017-01-14 14:00     ` Vladimir Davydov

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=20170117000754.GA25218@js1304-P5Q-DELUXE \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=cl@linux.com \
    --cc=hannes@cmpxchg.org \
    --cc=jsvana@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=tj@kernel.org \
    --cc=vdavydov@tarantool.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.