From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4D97C3A5A6 for ; Wed, 28 Aug 2019 11:43:10 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 88C6B22CED for ; Wed, 28 Aug 2019 11:43:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 88C6B22CED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 0835A6B0005; Wed, 28 Aug 2019 07:43:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 034B56B000D; Wed, 28 Aug 2019 07:43:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8C206B000E; Wed, 28 Aug 2019 07:43:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0182.hostedemail.com [216.40.44.182]) by kanga.kvack.org (Postfix) with ESMTP id C73796B0005 for ; Wed, 28 Aug 2019 07:43:09 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id 8090B824CA3D for ; Wed, 28 Aug 2019 11:43:09 +0000 (UTC) X-FDA: 75871650498.07.bean29_55ba145370d5b X-HE-Tag: bean29_55ba145370d5b X-Filterd-Recvd-Size: 9844 Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by imf40.hostedemail.com (Postfix) with ESMTP for ; Wed, 28 Aug 2019 11:43:08 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 057F3AF2C; Wed, 28 Aug 2019 11:43:06 +0000 (UTC) Date: Wed, 28 Aug 2019 13:43:05 +0200 From: Michal Hocko To: Daniel Vetter Cc: LKML , Linux MM , DRI Development , Jason Gunthorpe , Peter Zijlstra , Ingo Molnar , Andrew Morton , David Rientjes , Christian =?iso-8859-1?Q?K=F6nig?= , =?iso-8859-1?B?Suly9G1l?= Glisse , Masahiro Yamada , Wei Wang , Andy Shevchenko , Thomas Gleixner , Jann Horn , Feng Tang , Kees Cook , Randy Dunlap , Daniel Vetter Subject: Re: [PATCH 3/5] kernel.h: Add non_block_start/end() Message-ID: <20190828114305.GH28313@dhcp22.suse.cz> References: <20190826201425.17547-1-daniel.vetter@ffwll.ch> <20190826201425.17547-4-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20190826201425.17547-4-daniel.vetter@ffwll.ch> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon 26-08-19 22:14:23, Daniel Vetter wrote: > In some special cases we must not block, but there's not a > spinlock, preempt-off, irqs-off or similar critical section already > that arms the might_sleep() debug checks. Add a non_block_start/end() > pair to annotate these. >=20 > This will be used in the oom paths of mmu-notifiers, where blocking is > not allowed to make sure there's forward progress. Quoting Michal: >=20 > "The notifier is called from quite a restricted context - oom_reaper - > which shouldn't depend on any locks or sleepable conditionals. The code > should be swift as well but we mostly do care about it to make a forwar= d > progress. Checking for sleepable context is the best thing we could com= e > up with that would describe these demands at least partially." >=20 > Peter also asked whether we want to catch spinlocks on top, but Michal > said those are less of a problem because spinlocks can't have an > indirect dependency upon the page allocator and hence close the loop > with the oom reaper. >=20 > Suggested by Michal Hocko. >=20 > v2: > - Improve commit message (Michal) > - Also check in schedule, not just might_sleep (Peter) >=20 > v3: It works better when I actually squash in the fixup I had lying > around :-/ >=20 > v4: Pick the suggestion from Andrew Morton to give non_block_start/end > some good kerneldoc comments. I added that other blocking calls like > wait_event pose similar issues, since that's the other example we > discussed. >=20 > Cc: Jason Gunthorpe > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Andrew Morton > Cc: Michal Hocko > Cc: David Rientjes > Cc: "Christian K=F6nig" > Cc: Daniel Vetter > Cc: "J=E9r=F4me Glisse" > Cc: linux-mm@kvack.org > Cc: Masahiro Yamada > Cc: Wei Wang > Cc: Andy Shevchenko > Cc: Thomas Gleixner > Cc: Jann Horn > Cc: Feng Tang > Cc: Kees Cook > Cc: Randy Dunlap > Cc: linux-kernel@vger.kernel.org > Acked-by: Christian K=F6nig (v1) > Acked-by: Peter Zijlstra (Intel) > Signed-off-by: Daniel Vetter Acked-by: Michal Hocko Thanks and sorry for being mostly silent/slow in discussions here. ETOOBUSY. > --- > include/linux/kernel.h | 25 ++++++++++++++++++++++++- > include/linux/sched.h | 4 ++++ > kernel/sched/core.c | 19 ++++++++++++++----- > 3 files changed, 42 insertions(+), 6 deletions(-) >=20 > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 4fa360a13c1e..82f84cfe372f 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -217,7 +217,9 @@ extern void __cant_sleep(const char *file, int line= , int preempt_offset); > * might_sleep - annotation for functions that can sleep > * > * this macro will print a stack trace if it is executed in an atomic > - * context (spinlock, irq-handler, ...). > + * context (spinlock, irq-handler, ...). Additional sections where blo= cking is > + * not allowed can be annotated with non_block_start() and non_block_e= nd() > + * pairs. > * > * This is a useful debugging help to be able to catch problems early = and not > * be bitten later when the calling function happens to sleep when it = is not > @@ -233,6 +235,25 @@ extern void __cant_sleep(const char *file, int lin= e, int preempt_offset); > # define cant_sleep() \ > do { __cant_sleep(__FILE__, __LINE__, 0); } while (0) > # define sched_annotate_sleep() (current->task_state_change =3D 0) > +/** > + * non_block_start - annotate the start of section where sleeping is p= rohibited > + * > + * This is on behalf of the oom reaper, specifically when it is callin= g the mmu > + * notifiers. The problem is that if the notifier were to block on, fo= r example, > + * mutex_lock() and if the process which holds that mutex were to perf= orm a > + * sleeping memory allocation, the oom reaper is now blocked on comple= tion of > + * that memory allocation. Other blocking calls like wait_event() pose= similar > + * issues. > + */ > +# define non_block_start() \ > + do { current->non_block_count++; } while (0) > +/** > + * non_block_end - annotate the end of section where sleeping is prohi= bited > + * > + * Closes a section opened by non_block_start(). > + */ > +# define non_block_end() \ > + do { WARN_ON(current->non_block_count-- =3D=3D 0); } while (0) > #else > static inline void ___might_sleep(const char *file, int line, > int preempt_offset) { } > @@ -241,6 +262,8 @@ extern void __cant_sleep(const char *file, int line= , int preempt_offset); > # define might_sleep() do { might_resched(); } while (0) > # define cant_sleep() do { } while (0) > # define sched_annotate_sleep() do { } while (0) > +# define non_block_start() do { } while (0) > +# define non_block_end() do { } while (0) > #endif > =20 > #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0) > diff --git a/include/linux/sched.h b/include/linux/sched.h > index b6ec130dff9b..e8bb965f5019 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -980,6 +980,10 @@ struct task_struct { > struct mutex_waiter *blocked_on; > #endif > =20 > +#ifdef CONFIG_DEBUG_ATOMIC_SLEEP > + int non_block_count; > +#endif > + > #ifdef CONFIG_TRACE_IRQFLAGS > unsigned int irq_events; > unsigned long hardirq_enable_ip; > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 45dceec209f4..0d01c7994a9a 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3752,13 +3752,22 @@ static noinline void __schedule_bug(struct task= _struct *prev) > /* > * Various schedule()-time debugging checks and statistics: > */ > -static inline void schedule_debug(struct task_struct *prev) > +static inline void schedule_debug(struct task_struct *prev, bool preem= pt) > { > #ifdef CONFIG_SCHED_STACK_END_CHECK > if (task_stack_end_corrupted(prev)) > panic("corrupted stack end detected inside scheduler\n"); > #endif > =20 > +#ifdef CONFIG_DEBUG_ATOMIC_SLEEP > + if (!preempt && prev->state && prev->non_block_count) { > + printk(KERN_ERR "BUG: scheduling in a non-blocking section: %s/%d/%i= \n", > + prev->comm, prev->pid, prev->non_block_count); > + dump_stack(); > + add_taint(TAINT_WARN, LOCKDEP_STILL_OK); > + } > +#endif > + > if (unlikely(in_atomic_preempt_off())) { > __schedule_bug(prev); > preempt_count_set(PREEMPT_DISABLED); > @@ -3870,7 +3879,7 @@ static void __sched notrace __schedule(bool preem= pt) > rq =3D cpu_rq(cpu); > prev =3D rq->curr; > =20 > - schedule_debug(prev); > + schedule_debug(prev, preempt); > =20 > if (sched_feat(HRTICK)) > hrtick_clear(rq); > @@ -6641,7 +6650,7 @@ void ___might_sleep(const char *file, int line, i= nt preempt_offset) > rcu_sleep_check(); > =20 > if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && > - !is_idle_task(current)) || > + !is_idle_task(current) && !current->non_block_count) || > system_state =3D=3D SYSTEM_BOOTING || system_state > SYSTEM_RUNNI= NG || > oops_in_progress) > return; > @@ -6657,8 +6666,8 @@ void ___might_sleep(const char *file, int line, i= nt preempt_offset) > "BUG: sleeping function called from invalid context at %s:%d\n", > file, line); > printk(KERN_ERR > - "in_atomic(): %d, irqs_disabled(): %d, pid: %d, name: %s\n", > - in_atomic(), irqs_disabled(), > + "in_atomic(): %d, irqs_disabled(): %d, non_block: %d, pid: %d, name:= %s\n", > + in_atomic(), irqs_disabled(), current->non_block_count, > current->pid, current->comm); > =20 > if (task_stack_end_corrupted(current)) > --=20 > 2.23.0 >=20 --=20 Michal Hocko SUSE Labs