On 5/28/19 1:08 PM, Vladimir Davydov wrote:
>>  static void flush_memcg_workqueue(struct kmem_cache *s)
>>  {
>> +	/*
>> +	 * memcg_params.dying is synchronized using slab_mutex AND
>> +	 * memcg_kmem_wq_lock spinlock, because it's not always
>> +	 * possible to grab slab_mutex.
>> +	 */
>>  	mutex_lock(&slab_mutex);
>> +	spin_lock(&memcg_kmem_wq_lock);
>>  	s->memcg_params.dying = true;
>> +	spin_unlock(&memcg_kmem_wq_lock);
> I would completely switch from the mutex to the new spin lock -
> acquiring them both looks weird.
>
>>  	mutex_unlock(&slab_mutex);
>>  
>>  	/*

There are places where the slab_mutex is held and sleeping functions
like kvzalloc() are called. I understand that taking both mutex and
spinlocks look ugly, but converting all the slab_mutex critical sections
to spinlock critical sections will be a major undertaking by itself. So
I would suggest leaving that for now.

Cheers,
Longman