Messages in this thread | | | Subject | Re: [PATCH 3/3] mm/slub: Fix potential deadlock problem in slab_attr_store() | From | Waiman Long <> | Date | Tue, 11 Feb 2020 18:30:13 -0500 |
| |
On 2/10/20 6:10 PM, Andrew Morton wrote: > On Mon, 10 Feb 2020 17:14:31 -0500 Waiman Long <longman@redhat.com> wrote: > >>>> --- a/mm/slub.c >>>> +++ b/mm/slub.c >>>> @@ -5536,7 +5536,12 @@ static ssize_t slab_attr_store(struct kobject *kobj, >>>> if (slab_state >= FULL && err >= 0 && is_root_cache(s)) { >>>> struct kmem_cache *c; >>>> >>>> - mutex_lock(&slab_mutex); >>>> + /* >>>> + * Timeout after 100ms >>>> + */ >>>> + if (mutex_timed_lock(&slab_mutex, 100) < 0) >>>> + return -EBUSY; >>>> + >>> Oh dear. Surely there's a better fix here. Does slab really need to >>> hold slab_mutex while creating that sysfs file? Why? >>> >>> If the issue is two threads trying to create the same sysfs file >>> (unlikely, given that both will need to have created the same cache) >>> then can we add a new mutex specifically for this purpose? >>> >>> Or something else. >>> >> Well, the current code iterates all the memory cgroups to set the same >> value in all of them. I believe the reason for holding the slab mutex is >> to make sure that memcg hierarchy is stable during this iteration >> process. > But that is unrelated to creation of the sysfs file? > OK, I will take a closer look at that.
Cheers, Longman
| |