Re: [PATCH v3] mm: fix race between kmem_cache destroy, create and deactivate
From: Shakeel Butt
Date: Sun Jun 10 2018 - 10:53:12 EST
On Sat, Jun 9, 2018 at 3:20 AM Vladimir Davydov <vdavydov.dev@xxxxxxxxx> wrote:
> On Tue, May 29, 2018 at 05:12:04PM -0700, Shakeel Butt wrote:
> > The memcg kmem cache creation and deactivation (SLUB only) is
> > asynchronous. If a root kmem cache is destroyed whose memcg cache is in
> > the process of creation or deactivation, the kernel may crash.
> > Example of one such crash:
> > general protection fault: 0000 [#1] SMP PTI
> > CPU: 1 PID: 1721 Comm: kworker/14:1 Not tainted 4.17.0-smp
> > ...
> > Workqueue: memcg_kmem_cache kmemcg_deactivate_workfn
> > RIP: 0010:has_cpu_slab
> > ...
> > Call Trace:
> > ? on_each_cpu_cond
> > __kmem_cache_shrink
> > kmemcg_cache_deact_after_rcu
> > kmemcg_deactivate_workfn
> > process_one_work
> > worker_thread
> > kthread
> > ret_from_fork+0x35/0x40
> > To fix this race, on root kmem cache destruction, mark the cache as
> > dying and flush the workqueue used for memcg kmem cache creation and
> > deactivation.
> > @@ -845,6 +862,8 @@ void kmem_cache_destroy(struct kmem_cache *s)
> > if (unlikely(!s))
> > return;
> > + flush_memcg_workqueue(s);
> > +
> This should definitely help against async memcg_kmem_cache_create(),
> but I'm afraid it doesn't eliminate the race with async destruction,
> unfortunately, because the latter uses call_rcu_sched():
> <dereference destroyed cache>
> Can we somehow flush those pending rcu requests?
You are right and thanks for catching that. Now I am wondering if
synchronize_sched() just before flush_workqueue() should be enough.
Otherwise we might have to replace call_sched_rcu with
synchronize_sched() in kmemcg_deactivate_workfn which I would not
prefer as that would holdup the kmem_cache workqueue.
Paul, we have a situation something similar to the following pseudo code.
flag = true
If CPU0 has called already called call_rchu_sched(callback) then later
if CPU1 calls synchronize_sched(). Is there any guarantee that on
return from synchronize_sched(), the rcu callback scheduled by CPU0
has already been executed?