NACK/Cmnt: [SRU][J][PATCH 1/2] rcu-tasks: fix mismerge in trc_inspect_reader

[Stefan Bader]

On 22.11.24 10:46, Krister Johansen wrote:

I would in this case change the subject/commit title to:

[SRU][J][PATCH 1/2] UBUNTU: SAUCE: rcu-tasks: fix mismerge in 
trc_inspect_reader

BugLink: https://bugs.launchpad.net/bugs/2089373

> Upstream converted the '!nesting' to 'nesting ? -EINVAL : 0' as part of a manual
> conflict resolution during their merge of 9b3c4ab304("sched,rcu: Rework
> try_invoke_on_locked_down_task()").
> 
> When this change was pulled into the Ubuntu trees the resulting conflict was
> incorrectly resolved when the patch was cherry-picked.  Continuing to use
> '!nesting' results in trc_inspect_reader erroneously flagging tasks as holdouts
> when they are actually quiescent.  This subsequently results in IPIs being sent
> to CPUs where one is not needed.  Additionally, on platforms that have CPU
> hotplug slots empty, this also results in WARNs about errors sending IPIs to
> invalid CPUs.  Resolving the conflict in the same fashion as linux has addresses
> this problem.
> 

Fixes: 0f17585efd61 "rcu-tasks: Add trc_inspect_reader() checks for 
exiting critical section"
(backported from commit 6fedc28076bbbb32edb722e80f9406a3d1d668a8)
> Signed-off-by: Krister Johansen <kjlx at templeofstupid.com>

That would better show the history of things. The BugLink (note the 
preferred short form also has to be included into each submission. Could 
you re-send this as a new v2 thread?
Thanks,

-Stefan
> ---
>   kernel/rcu/tasks.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index b9f762c2b083..5528c172570b 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -982,7 +982,7 @@ static int trc_inspect_reader(struct task_struct *t, void *arg)
>   	// holdout list.
>   	t->trc_reader_checked = nesting >= 0;
>   	if (nesting <= 0)
> -		return !nesting;  // If in QS, done, otherwise try again later.
> +		return nesting ? -EINVAL : 0;  // If in QS, done, otherwise try again later.
>   
>   	// The task is in a read-side critical section, so set up its
>   	// state so that it will awaken the grace-period kthread upon exit


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 48643 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20241122/2b695ec7/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20241122/2b695ec7/attachment-0001.sig>