Re: [PATCH V5 06/11] entry: Prevent DEBUG_PREEMPT warning

From: Peter Zijlstra <peterz@infradead.org>
To: guoren@kernel.org
Cc: arnd@arndb.de, palmer@rivosinc.com, tglx@linutronix.de,
	luto@kernel.org, conor.dooley@microchip.com, heiko@sntech.de,
	jszhang@kernel.org, lazyparser@gmail.com, falcon@tinylab.org,
	chenhuacai@kernel.org, apatel@ventanamicro.com,
	atishp@atishpatra.org, palmer@dabbelt.com,
	paul.walmsley@sifive.com, mark.rutland@arm.com,
	zouyipeng@huawei.com, bigeasy@linutronix.de,
	David.Laight@aculab.com, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH V5 06/11] entry: Prevent DEBUG_PREEMPT warning
Date: Mon, 19 Sep 2022 13:58:53 +0200	[thread overview]
Message-ID: <YyhZfUi17TEaOLWv@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20220918155246.1203293-7-guoren@kernel.org>

On Sun, Sep 18, 2022 at 11:52:41AM -0400, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> When DEBUG_PREEMPT=y,
> 	exit_to_user_mode_prepare
> 	->tick_nohz_user_enter_prepare
> 	  ->tick_nohz_full_cpu(smp_processor_id())
> 	    ->smp_processor_id()
> 	      ->debug_smp_processor_id()
> 		->check preempt_count() then:
> 
> [    5.717610] BUG: using smp_processor_id() in preemptible [00000000]
> code: S20urandom/94
> [    5.718111] caller is debug_smp_processor_id+0x24/0x38
> [    5.718417] CPU: 1 PID: 94 Comm: S20urandom Not tainted
> 6.0.0-rc3-00010-gfd0a0d619c63-dirty #238
> [    5.718886] Hardware name: riscv-virtio,qemu (DT)
> [    5.719136] Call Trace:
> [    5.719281] [<ffffffff800055fc>] dump_backtrace+0x2c/0x3c
> [    5.719566] [<ffffffff80ae6cb0>] show_stack+0x44/0x5c
> [    5.720023] [<ffffffff80aee870>] dump_stack_lvl+0x74/0xa4
> [    5.720557] [<ffffffff80aee8bc>] dump_stack+0x1c/0x2c
> [    5.721033] [<ffffffff80af65c0>]
> check_preemption_disabled+0x104/0x108
> [    5.721538] [<ffffffff80af65e8>] debug_smp_processor_id+0x24/0x38
> [    5.722001] [<ffffffff800aee30>] exit_to_user_mode_prepare+0x48/0x178
> [    5.722355] [<ffffffff80af5bf4>] irqentry_exit_to_user_mode+0x18/0x30
> [    5.722685] [<ffffffff80af5c70>] irqentry_exit+0x64/0xa4
> [    5.722953] [<ffffffff80af52f4>] do_page_fault+0x1d8/0x544
> [    5.723291] [<ffffffff80003310>] ret_from_exception+0x0/0xb8
> 
> (Above is found in riscv platform with generic_entry)
> 
> The smp_processor_id() needs irqs disable or preempt_disable, so use
> preempt dis/in protecting the tick_nohz_user_enter_prepare().
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> ---
>  kernel/entry/common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index 063068a9ea9b..36e4cd50531c 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -194,8 +194,10 @@ static void exit_to_user_mode_prepare(struct pt_regs *regs)
>  
>  	lockdep_assert_irqs_disabled();

    Observe ^^^^

>  
> +	preempt_disable();
>  	/* Flush pending rcuog wakeup before the last need_resched() check */
>  	tick_nohz_user_enter_prepare();
> +	preempt_enable();

This makes no sense; if IRQs are disabled, check_preemption_disabled()
should bail early per:

	if (irqs_disabled())
		goto out;

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv