Re: [PATCH] mm,oom: Teach lockdep about oom_lock.

[Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>]

On 2019/03/13 0:31, Michal Hocko wrote:
>> @@ -1120,8 +1129,25 @@ void pagefault_out_of_memory(void)
>>  	if (mem_cgroup_oom_synchronize(true))
>>  		return;
>>  
>> -	if (!mutex_trylock(&oom_lock))
>> +	if (!mutex_trylock(&oom_lock)) {
>> +		/*
>> +		 * This corresponds to prepare_alloc_pages(). Lockdep will
>> +		 * complain if e.g. OOM notifier for global OOM by error
>> +		 * triggered pagefault OOM path.
>> +		 */
>> +		oom_reclaim_acquire(GFP_KERNEL);
>> +		oom_reclaim_release(GFP_KERNEL);
>>  		return;
>> +	}
>> +	/*
>> +	 * Teach lockdep to consider that current thread is not allowed to
>> +	 * involve (even indirectly via dependency) __GFP_DIRECT_RECLAIM &&
>> +	 * !__GFP_NORETRY allocation from this function, for such allocation
>> +	 * will have to wait for completion of this function when
>> +	 * __alloc_pages_may_oom() is called.
>> +	 */
>> +	oom_reclaim_release(GFP_KERNEL);
>> +	oom_reclaim_acquire(GFP_KERNEL);
> 
> This part is not really clear to me. Why do you release&acquire when
> mutex_trylock just acquire the lock? If this is really needed then this
> should be put into the comment.

I think there is a reason lockdep needs to distinguish trylock and lock.
I don't know how lockdep utilizes "trylock or lock" information upon validation, but
explicitly telling lockdep that "oom_lock acts as if held by lock" should not harm.

#define mutex_acquire(l, s, t, i)               lock_acquire_exclusive(l, s, t, NULL, i)
#define lock_acquire_exclusive(l, s, t, n, i)           lock_acquire(l, s, t, 0, 1, n, i)
void lock_acquire(struct lockdep_map *lock, unsigned int subclass, int trylock, int read, int check, struct lockdep_map *nest_lock, unsigned long ip);

> 
>>  	out_of_memory(&oc);
>>  	mutex_unlock(&oom_lock);
>>  }
>