linux-kernel - Re: [PATCH 5.15 000/183] 5.15.134-rc1 review

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <273869c3-ba4b-4173-a14e-bd201d900079@paulmck-laptop>
Date:   Wed, 11 Oct 2023 09:31:35 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Joel Fernandes <joel@...lfernandes.org>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, patches@...ts.linux.dev,
        linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
        akpm@...ux-foundation.org, linux@...ck-us.net, shuah@...nel.org,
        patches@...nelci.org, lkft-triage@...ts.linaro.org, pavel@...x.de,
        jonathanh@...dia.com, f.fainelli@...il.com,
        sudipm.mukherjee@...il.com, srw@...dewatkins.net, rwarsow@....de,
        conor@...nel.org, Chengming Zhou <zhouchengming@...edance.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ovidiu Panait <ovidiu.panait@...driver.com>,
        Ingo Molnar <mingo@...nel.org>, rcu <rcu@...r.kernel.org>
Subject: Re: [PATCH 5.15 000/183] 5.15.134-rc1 review

On Wed, Oct 11, 2023 at 03:47:23PM +0200, Frederic Weisbecker wrote:
> Le Tue, Oct 10, 2023 at 06:34:35PM -0700, Paul E. McKenney a écrit :
> > If this problem is real, fixes include:
> > 
> > o	Revert Liam's patch and make Tiny RCU's call_rcu() deal with
> > 	the problem.  This is overhead and non-tinyness, but to Joel's
> > 	point, it might be best.
> 
> But what is calling call_rcu() or start_poll_synchronize_rcu() so
> early that the CPU is not even online? (that's before boot_cpu_init() !)
> 
> Deferring PF_IDLE setting might pave the way for more issues like this one,
> present or future. Though is_idle_task() returning true when the task is not
> in the idle loop but is playing the init/0 role is debatable.
> 
> An alternative for tiny RCU is to force waking up ksoftirqd when call_rcu()
> is in the idle task. Since rcu_qs() during the context switch raises a softirq
> anyway. It's more overhead for start_poll_synchronize_rcu() though but do we
> expect much RCU polling in idle?

Nice!!!

This does solve the original problem with little or no additional overhead
(perhaps even with decreased overhead), and avoids the other RCU Tasks
issues.

						Thanx, Paul

> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a92bce40b04b..6ab15233e2be 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -604,6 +604,7 @@ extern void __raise_softirq_irqoff(unsigned int nr);
>  
>  extern void raise_softirq_irqoff(unsigned int nr);
>  extern void raise_softirq(unsigned int nr);
> +extern void raise_ksoftirqd_irqsoff(unsigned int nr);
>  
>  DECLARE_PER_CPU(struct task_struct *, ksoftirqd);
>  
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index 42f7589e51e0..872dab8b8b53 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -189,12 +189,12 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
>  	local_irq_save(flags);
>  	*rcu_ctrlblk.curtail = head;
>  	rcu_ctrlblk.curtail = &head->next;
> -	local_irq_restore(flags);
>  
>  	if (unlikely(is_idle_task(current))) {
>  		/* force scheduling for rcu_qs() */
> -		resched_cpu(0);
> +		raise_ksoftirqd_irqsoff(RCU_SOFTIRQ);
>  	}
> +	local_irq_restore(flags);
>  }
>  EXPORT_SYMBOL_GPL(call_rcu);
>  
> @@ -225,10 +225,13 @@ EXPORT_SYMBOL_GPL(get_state_synchronize_rcu);
>  unsigned long start_poll_synchronize_rcu(void)
>  {
>  	unsigned long gp_seq = get_state_synchronize_rcu();
> +	unsigned long flags;
>  
>  	if (unlikely(is_idle_task(current))) {
> +		local_irq_save(flags);
>  		/* force scheduling for rcu_qs() */
> -		resched_cpu(0);
> +		raise_ksoftirqd_irqsoff(RCU_SOFTIRQ);
> +		local_irq_restore(flags);
>  	}
>  	return gp_seq;
>  }
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 210cf5f8d92c..ef105cbdc705 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -695,6 +695,14 @@ void __raise_softirq_irqoff(unsigned int nr)
>  	or_softirq_pending(1UL << nr);
>  }
>  
> +#ifdef CONFIG_RCU_TINY
> +void raise_ksoftirqd(unsigned int nr)
> +{
> +	__raise_softirq_irqoff(nr);
> +	wakeup_softirqd();
> +}
> +#endif
> +
>  void open_softirq(int nr, void (*action)(struct softirq_action *))
>  {
>  	softirq_vec[nr].action = action;
> 
> 
> 
>