Received-SPF: pass (google.com: domain of linux-kernel+bounces-213390-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Wei Fu <fuweid89@gmail.com>
To: ebiederm@xmission.com
Cc: Sudhanva.Huruli@microsoft.com,
	akpm@linux-foundation.org,
	apais@linux.microsoft.com,
	axboe@kernel.dk,
	boqun.feng@gmail.com,
	brauner@kernel.org,
	frederic@kernel.org,
	fuweid89@gmail.com,
	j.granados@samsung.com,
	jiangshanlai@gmail.com,
	joel@joelfernandes.org,
	josh@joshtriplett.org,
	linux-kernel@vger.kernel.org,
	mathieu.desnoyers@efficios.com,
	michael.christie@oracle.com,
	mjguzik@gmail.com,
	neeraj.upadhyay@kernel.org,
	oleg@redhat.com,
	paulmck@kernel.org,
	qiang.zhang1211@gmail.com,
	rachelmenge@linux.microsoft.com,
	rcu@vger.kernel.org,
	rostedt@goodmis.org
Subject: Re: [PATCH] zap_pid_ns_processes: clear TIF_NOTIFY_SIGNAL along with TIF_SIGPENDING
Date: Thu, 13 Jun 2024 22:02:16 +0800
Message-ID: <20240613140216.30327-1-fuweid89@gmail.com>
In-Reply-To: <87a5jpqamx.fsf@email.froward.int.ebiederm.org>
References: <87a5jpqamx.fsf@email.froward.int.ebiederm.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

> 
> Oleg Nesterov <oleg@redhat.com> writes:
> 
> > kernel_wait4() doesn't sleep and returns -EINTR if there is no
> > eligible child and signal_pending() is true.
> >
> > That is why zap_pid_ns_processes() clears TIF_SIGPENDING but this is not
> > enough, it should also clear TIF_NOTIFY_SIGNAL to make signal_pending()
> > return false and avoid a busy-wait loop.
> 
> I took a look through the code.  It used to be that TIF_NOTIFY_SIGNAL
> was all about waking up a task so that task_work_run can be used.
> io_uring still mostly uses it that way.  There is also a use in
> kthread_stop that just uses it as a TIF_SIGPENDING without having a
> pending signal.
> 
> At the point in do_exit where exit_notify and thus zap_pid_ns_processes
> is called I can't possibly see a use for TIF_NOTIFY_SIGNAL.
> exit_task_work, exit_signals, and io_uring_cancel have all been called.
> 
> So TIF_NOTIFY_SIGNAL should be spurious at this point and safe to clear.
> Why it remains set is a mystery to me.

I think there is a case that TIF_NOTIFY_SIGNAL remains set.

Init process has main-thread, sub-thread-X and iou-wrk-thread-X (created by
sub-thread-X). When main-thread enters exit_group, both sub-thread-X and
iou-wrk-thread-X are set by TIF_SIGPENDING and wake up. The sub-thread-X could
call io_uring_cancel to set TIF_NOTIFY_SIGNAL for iou-wrk-thread-X which doesn't
have chance to clear it. And then iou-wrk-thread-X gets into zap_pid_ns_processes
function with TIF_NOTIFY_SIGNAL flag. If there are active processes in that pid
namespace, it will run into this issue.

Wei