From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by kanga.kvack.org (Postfix) with ESMTP id 4D8F06B303A for ; Fri, 23 Nov 2018 03:46:14 -0500 (EST) Received: by mail-ed1-f71.google.com with SMTP id c53so5583706edc.9 for ; Fri, 23 Nov 2018 00:46:14 -0800 (PST) Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id k37sor6656eda.26.2018.11.23.00.46.12 for (Google Transport Security); Fri, 23 Nov 2018 00:46:12 -0800 (PST) Date: Fri, 23 Nov 2018 09:46:09 +0100 From: Daniel Vetter Subject: Re: [PATCH 2/3] mm, notifier: Catch sleeping/blocking for !blockable Message-ID: <20181123084609.GH4266@phenom.ffwll.local> References: <20181122165106.18238-1-daniel.vetter@ffwll.ch> <20181122165106.18238-3-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: "Koenig, Christian" Cc: Daniel Vetter , LKML , Linux MM , Intel Graphics Development , DRI Development , Andrew Morton , Michal Hocko , David Rientjes , =?iso-8859-1?B?Suly9G1l?= Glisse , Daniel Vetter On Thu, Nov 22, 2018 at 06:55:17PM +0000, Koenig, Christian wrote: > Am 22.11.18 um 17:51 schrieb Daniel Vetter: > > We need to make sure implementations don't cheat and don't have a > > possible schedule/blocking point deeply burried where review can't > > catch it. > > > > I'm not sure whether this is the best way to make sure all the > > might_sleep() callsites trigger, and it's a bit ugly in the code flow. > > But it gets the job done. > > > > Cc: Andrew Morton > > Cc: Michal Hocko > > Cc: David Rientjes > > Cc: "Christian K�nig" > > Cc: Daniel Vetter > > Cc: "J�r�me Glisse" > > Cc: linux-mm@kvack.org > > Signed-off-by: Daniel Vetter > > --- > > mm/mmu_notifier.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c > > index 59e102589a25..4d282cfb296e 100644 > > --- a/mm/mmu_notifier.c > > +++ b/mm/mmu_notifier.c > > @@ -185,7 +185,13 @@ int __mmu_notifier_invalidate_range_start(struct mm_struct *mm, > > id = srcu_read_lock(&srcu); > > hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) { > > if (mn->ops->invalidate_range_start) { > > - int _ret = mn->ops->invalidate_range_start(mn, mm, start, end, blockable); > > + int _ret; > > + > > + if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP) && !blockable) > > + preempt_disable(); > > + _ret = mn->ops->invalidate_range_start(mn, mm, start, end, blockable); > > + if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP) && !blockable) > > + preempt_enable(); > > Just for the sake of better documenting this how about adding this to > include/linux/kernel.h right next to might_sleep(): > > #define disallow_sleeping_if(cond)��� for((cond) ? preempt_disable() : > (void)0; (cond); preempt_disable()) > > (Just from the back of my head, might contain peanuts and/or hints of > errors). I think these magic for blocks aren't used in the kernel. goto breaks them, and we use goto a lot. I think a disallow/allow_sleep() pair with the conditional preept_disable/enable() calls would be nice though. I can do that if the overall idea sticks. -Daniel > > Christian. > > > if (_ret) { > > pr_info("%pS callback failed with %d in %sblockable context.\n", > > mn->ops->invalidate_range_start, _ret, > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch