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);
>> }
>
