All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Thomas Gleixner <tglx@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Kees Cook <kees@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	John Stultz <jstultz@google.com>, Ingo Molnar <mingo@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, imv4bel@gmail.com
Subject: Re: [PATCH v2] posix-cpu-timers: Dequeue per-thread timers before exchange_tids()
Date: Thu, 27 Aug 2026 22:13:01 +0900	[thread overview]
Message-ID: <apA33QXNSwDBxeXK@v4bel> (raw)
In-Reply-To: <ao7iQ_wOLghOvTsq@redhat.com>

On Wed, Aug 26, 2026 at 02:55:31PM +0200, Oleg Nesterov wrote:
> OMG, my head spins ;) I'll try to read your explanation tomorrow.
> Right now I am all confused.
> 
> But let me ask some stupid questions right now, please help me to
> understand the problem.
> 
> On 08/26, Hyunwoo Kim wrote:
> >
> > begin_new_exec() calls posix_cpu_timers_exit(me) right after
> > exec_task_namespaces() and that removes the leftover node, so the state
> > normally stays invisible. But bprm->point_of_no_return is set before
> > de_thread(), so if unshare_files(), set_mm_exe_file(), exec_mmap() or
> > exec_task_namespaces() fails, the task dies before it gets there.
> 
> Do I understand this correctly? If begin_new_exec() does call
> posix_cpu_timers_exit(me), then everything is fine.

Yes. A failure after it, e.g. unshare_sighand(), is harmless.

> 
> Yes? If yes
> 
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -1003,6 +1003,18 @@ static int de_thread(struct task_struct *tsk)
> >  		 * the former thread group leader:
> >  		 */
> >
> > +#ifdef CONFIG_POSIX_TIMERS
> > +		/*
> > +		 * exchange_tids() hands this thread's PID to the old leader,
> > +		 * which is reaped right after. The PID lookup in
> > +		 * timer_lock_sighand() then fails while the per thread CPU
> > +		 * timers are still queued here, so dequeue them first.
> > +		 */
> > +		spin_lock(lock);
> > +		posix_cpu_timers_exit(tsk);
> > +		spin_unlock(lock);
> > +#endif
> 
> ... then why do we need to call posix_cpu_timers_exit(current) before
> exchange_tids() ?

Strictly we don't. posix_cpu_timers_exit() walks tsk->posix_cputimers
directly and never does the PID lookup, so it only has to happen before
exit_itimers()

I put it before exchange_tids() so that the state timer_lock_sighand()
warns about never exists in the first place.

> 
> And what if another process attaches another cpu timer to the execing
> thread right after the code above?

Not for a per-thread timer, pid_for_clock() requires same_thread_group()
and there is no other thread left at that point.

A process wide timer has no such check, but it goes to
signal->posix_cputimers and the TGID lookup stays valid, so it isn't
affected.

> 
> Can we move posix_cpu_timers_exit() or the whole CONFIG_POSIX_TIMERS
> sequence in begin_new_exec() up, right after de_thread() before the
> "if (retval)" check?

posix_cpu_timers_exit(me) there is enough. I haven't tested moving the 
whole block.


Best regards,
Hyunwoo Kim


  reply	other threads:[~2026-08-27 13:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26 11:41 [PATCH v2] posix-cpu-timers: Dequeue per-thread timers before exchange_tids() Hyunwoo Kim
2026-08-26 12:55 ` Oleg Nesterov
2026-08-27 13:13   ` Hyunwoo Kim [this message]
2026-08-31 16:20   ` Thomas Gleixner
2026-08-31 18:24     ` Oleg Nesterov
2026-09-01 14:50       ` Thomas Gleixner
2026-09-01 15:11         ` Thomas Gleixner
2026-08-31 11:56 ` Thomas Gleixner
2026-08-31 16:19   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apA33QXNSwDBxeXK@v4bel \
    --to=imv4bel@gmail.com \
    --cc=anna-maria@linutronix.de \
    --cc=brauner@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=frederic@kernel.org \
    --cc=jack@suse.cz \
    --cc=jstultz@google.com \
    --cc=kees@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.