From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752826AbdCAKfV (ORCPT ); Wed, 1 Mar 2017 05:35:21 -0500 Received: from mx2.suse.de ([195.135.220.15]:58558 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752723AbdCAKeM (ORCPT ); Wed, 1 Mar 2017 05:34:12 -0500 Subject: Re: [PATCH v2] lockdep: Teach lockdep about memalloc_noio_save To: Peter Zijlstra References: <1488354506-23774-1-git-send-email-nborisov@suse.com> <1488355140-24528-1-git-send-email-nborisov@suse.com> <20170301094657.GC6515@twins.programming.kicks-ass.net> Cc: mingo@redhat.com, linux-kernel@vger.kernel.org, ming.lei@canonical.com, minchan@kernel.org, mel@csn.ul.ie From: Nikolay Borisov Message-ID: <8ec973ad-ab96-ed0d-25ee-c4bee9ab5841@suse.com> Date: Wed, 1 Mar 2017 11:57:13 +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: <20170301094657.GC6515@twins.programming.kicks-ass.net> 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 11:46, Peter Zijlstra wrote: > On Wed, Mar 01, 2017 at 09:59:00AM +0200, 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 > > I'm not up to date on the specific, but GFP_IO is separate from GFP_FS. > > And MEMALLOC_NOIO only clears GFP_IO but leaves GFP_FS set. static inline gfp_t memalloc_noio_flags(gfp_t flags) { if (unlikely(current->flags & PF_MEMALLOC_NOIO)) flags &= ~(__GFP_IO | __GFP_FS); return flags; } > > Therefore I think your change is wrong, but I might have overlooked a > detail. Added original authors to Cc to clarify. > > >> Signed-off-by: Nikolay Borisov >> --- >> kernel/locking/lockdep.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> >> Turned out there was another place where RELCAIM_FS was being set, fix it >> by using the memalloc_noio_flags when setting ->lockdep_reclaim_gfp flags. >> >> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >> index 9812e5dd409e..87cf9910e66f 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) >> return; >> >> /* We're only interested __GFP_FS allocations for now */ >> @@ -3852,7 +3853,7 @@ EXPORT_SYMBOL_GPL(lock_unpin_lock); >> >> void lockdep_set_current_reclaim_state(gfp_t gfp_mask) >> { >> - current->lockdep_reclaim_gfp = gfp_mask; >> + current->lockdep_reclaim_gfp = memalloc_noio_flags(gfp_mask); >> } >> >> void lockdep_clear_current_reclaim_state(void) >> -- >> 2.7.4 >> >