From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752428AbdCALJF (ORCPT ); Wed, 1 Mar 2017 06:09:05 -0500 Received: from mx2.suse.de ([195.135.220.15]:33130 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750703AbdCALIr (ORCPT ); Wed, 1 Mar 2017 06:08:47 -0500 Subject: Re: [PATCH] lockdep: Teach lockdep about memalloc_noio_save To: Michal Hocko , Vlastimil Babka References: <1488354506-23774-1-git-send-email-nborisov@suse.com> <20170301103131.GD1124@dhcp22.suse.cz> Cc: peterz@infradead.org, mingo@redhat.com, linux-kernel@vger.kernel.org From: Nikolay Borisov Message-ID: <785b681e-ce94-1556-b662-347126be5253@suse.com> Date: Wed, 1 Mar 2017 13:07:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170301103131.GD1124@dhcp22.suse.cz> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1.03.2017 12:31, Michal Hocko wrote: > On Wed 01-03-17 11:22:51, Vlastimil Babka wrote: >> On 03/01/2017 08:48 AM, Nikolay Borisov wrote: >>> Commit 21caf2fc1931 ("mm: teach mm by current context info to not do I/O >>> during memory allocation") added the memalloc_noio_(save|restore) functions >>> to enable people to modify the MM behavior by disbaling I/O during memory >>> allocation. This prevents allocation paths recursing back into the filesystem >>> without explicitly changing the flags for every allocation site. Yet, lockdep >>> not being aware of that is prone to showing false positives. Fix this >>> by teaching it that the presence of PF_MEMALLOC_NOIO flag mean we are not >>> going to issue any I/O >>> >>> Signed-off-by: Nikolay Borisov >>> --- >>> kernel/locking/lockdep.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >>> index 9812e5dd409e..5715fdcede28 100644 >>> --- a/kernel/locking/lockdep.c >>> +++ b/kernel/locking/lockdep.c >>> @@ -2866,7 +2866,8 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags) >>> return; >>> >>> /* this guy won't enter reclaim */ >>> - if ((curr->flags & PF_MEMALLOC) && !(gfp_mask & __GFP_NOMEMALLOC)) >>> + if (((curr->flags & PF_MEMALLOC) && !(gfp_mask & __GFP_NOMEMALLOC)) || >>> + curr->flags & PF_MEMALLOC_NOIO) >> >> It would be slightly better to use memalloc_noio_flags() here. Michal is >> planning to convert it to take also a new PF_MEMALLOC_NOFS flag into >> account, and there would be less chance of forgetting to update this place. > > Yes, you are right. The following should do the trick. I am really > surprised we haven't noticed this before. I thought we were shaving the > gfp_mask before the allocator goes the lockdep_trace_alloc way. But it > is not and what is worse SLAB tracks this as well so we cannot rely on > the proper gfp mask. The positive thing is that the recursion avoidance > works because we always clear GFP_IO and GFP_FS when doing reclaim. Okay I will send a revised patch, doing it the way you suggested. > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index 7c38f8f3d97b..0c70b26849ce 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -2861,6 +2861,8 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags) > if (unlikely(!debug_locks)) > return; > > + gfp_mask = memalloc_noio_flags(gfp_mask); > + > /* no reclaim without waiting on it */ > if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) > return; >