Re: [PATCH v3 3/6] exit: postpone tty_kref_put() until after tasklist_lock is dropped

Oleg Nesterov
2025-02-04T11:23:14Z

On 02/03, Mateusz Guzik wrote:
>
> On Mon, Feb 3, 2025 at 7:07 PM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 02/01, Mateusz Guzik wrote:
> > >
> > > Instead of smuggling the tty pointer directly, use a struct so that more
> > > things can be added later.
> >
> > I am not sure this particular change worth the effort, but I won't argue.
> > I'd like to know what Eric thinks.
> >
>
> it trivially whacks an atomic from an area protected by a global lock

Yes, but I am not sure this can make a noticeable difference... Let
me repeat, I am not really arguing, I leave this to you and Eric.
The patch looks correct and I have already acked it. Just it looks
"less obvious" to me than other changes in this series.

And even if we do this, I am not sure we need the new
"struct release_task_post", it seems we can simply move
tty_kref_put() from __exit_signal() to release_task(), see the
patch at the end.

Nobody can touch signal->tty after the group leader passes __exit_signal(),
if nothing else lock_task_sighand() can't succeed.

But again, feel free to ignore.

Oleg.

--- x/kernel/exit.c
+++ x/kernel/exit.c
@@ -146,7 +146,6 @@ static void __exit_signal(struct task_st
struct signal_struct *sig = tsk->signal;
bool group_dead = thread_group_leader(tsk);
struct sighand_struct *sighand;
- struct tty_struct *tty;
u64 utime, stime;

sighand = rcu_dereference_check(tsk->sighand,
@@ -159,10 +158,7 @@ static void __exit_signal(struct task_st
posix_cpu_timers_exit_group(tsk);
#endif

- if (group_dead) {
- tty = sig->tty;
- sig->tty = NULL;
- } else {
+ if (!group_dead) {
/*
* If there is any task waiting for the group exit
* then notify it:
@@ -210,10 +206,8 @@ static void __exit_signal(struct task_st

__cleanup_sighand(sighand);
clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
- if (group_dead) {
+ if (group_dead)
flush_sigqueue(&sig->shared_pending);
- tty_kref_put(tty);
- }
}

static void delayed_put_task_struct(struct rcu_head *rhp)
@@ -279,6 +273,10 @@ repeat:
proc_flush_pid(thread_pid);
put_pid(thread_pid);
release_thread(p);
+
+ if (p == leader)
+ tty_kref_put(p->signal->tty);
+
put_task_struct_rcu_user(p);

p = leader;