linux-kernel - Re: [RESEND PATCH] sched: restore the behavior of put_task

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: <20251017143944.GB21102@redhat.com>
Date: Fri, 17 Oct 2025 16:39:45 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: "Luis Claudio R. Goncalves" <lgoncalv@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Clark Williams <clrkwllms@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>, Tejun Heo <tj@...nel.org>,
	David Vernet <dvernet@...a.com>, Barret Rhoden <brho@...gle.com>,
	Josh Don <joshdon@...gle.com>, Crystal Wood <crwood@...hat.com>,
	linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Juri Lelli <juri.lelli@...hat.com>, Ben Segall <bsegall@...gle.com>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Ingo Molnar <mingo@...hat.com>, Mel Gorman <mgorman@...e.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Wander Lairson Costa <wander@...hat.com>
Subject: Re: [RESEND PATCH] sched: restore the behavior of put_task_struct()
 for non-rt

Peter,

So I think that delayed_put_task_struct() has to stay and this means
that we have 2 RCU grace periods in the "typical" case when the exiting
task without extra references is reaped (or autoreaped if a subthread).

As I said I won't really argue, but what is your final thought on this?
Should we forget this patch from Luis? I'd prefer to restore the !RT
exception...

And another question. Sasha and Greg are going to add 8671bad873eb
("sched: Do not call __put_task_struct() on rt if pi_blocked_on is set")
to -stable. Do you think it is fine to backport this commit as is?

Oleg.

On 09/16, Oleg Nesterov wrote:
>
> On 09/16, Peter Zijlstra wrote:
> >
> > On Mon, Sep 15, 2025 at 02:35:40PM +0200, Oleg Nesterov wrote:
> > > On 09/15, Peter Zijlstra wrote:
> > > >
> > > > Why have !RT behave differently? That is, why isn't this simply a
> > > > 'buggy' comment/changelog issue?
> > >
> > > Well, this was discussed several times, in particular see
> > > https://lore.kernel.org/all/CAHk-=whtj+aSYftniMRG4xvFE8dmmYyrqcJyPmzStsfj5w9r=w@mail.gmail.com/
> > >
> > > And task_struct->rcu_users was introduced to avoid RCU call in put_task_struct() ...
> >
> > Ah, I forgot about that thing.. Although I had vague memories of that
> > argument on rcu_assign_pointer() vs RCU_INIT_POINTER().
> >
> > > But I won't really argue if you decide to remove this !RT optimization, although
> > > I think it would be better to do this in a separate patch.
> >
> > Right. So when they wanted to remove that preemptible() clause, I was
> > like why again do we have this !RT exception at all, and can't we get
> > rid of that.
> >
> > If git isn't confusing me again, this got merged in this cycle. But so
> > far no benchmark came and told us this was a bad idea.
>
> I still think it would be safer to merge this patch from Luis before
> v6.17, then possibly remove this special case in a separate patch...
>
> > So what do we do now... do we restore the !RT exception (so far there
> > aren't any numbers to suggest this mattered) or do we let it be for a
> > bit and then go and clean things up?
>
> It is not that simple. Please note that put_task_struct_rcu_user()
> delays put(tsk->usage), not free(tsk).
>
> So for example with this change
>
> > @@ -305,7 +288,7 @@ void release_task(struct task_struct *p)
> >  	if (thread_group_leader(p))
> >  		flush_sigqueue(&p->signal->shared_pending);
> >
> > -	put_task_struct_rcu_user(p);
> > +	put_task_struct(p);
> >
> >  	p = leader;
> >  	if (unlikely(zap_leader))
>
> This code
>
> 	rcu_read_lock();
> 	tsk = find_task_by_vpid(...);
> 	if (tsk)
> 		get_task_struct(tsk);
> 	rcu_read_unlock();
>
> becomes wrong, get_task_struct(tsk) can increment tsk->usage == 0.
>
> Oleg.