Re: [PATCH 5/5] task_work: use TIF_NOTIFY_SIGNAL if available

From: Jens Axboe <axboe@kernel.dk>
To: Thomas Gleixner <tglx@linutronix.de>, Oleg Nesterov <oleg@redhat.com>
Cc: linux-kernel@vger.kernel.org, io-uring@vger.kernel.org,
	peterz@infradead.org, Roman Gershman <romger@amazon.com>
Subject: Re: [PATCH 5/5] task_work: use TIF_NOTIFY_SIGNAL if available
Date: Fri, 16 Oct 2020 08:22:58 -0600	[thread overview]
Message-ID: <5b04c6c1-28df-4810-8382-f9a418d72267@kernel.dk> (raw)
In-Reply-To: <87d01itg4z.fsf@nanos.tec.linutronix.de>

On 10/16/20 8:11 AM, Thomas Gleixner wrote:
> Jens,
> 
> On Fri, Oct 16 2020 at 07:33, Jens Axboe wrote:
>> On 10/16/20 3:00 AM, Thomas Gleixner wrote:
>> I totally agree, and we're on the same page. I think you'll find that in
>> the past I always carry through, the task_work notification was somewhat
>> of a rush due to a hang related to it. For this particular case, the
>> cleanups and arch additions are pretty much ready to go.
> 
> As we seem to be on the same page with this, let me suggest how this
> should go:
> 
> 1) A cleanup for the task_work_add() mess. This is trivial enough and
>    should go in before rc1.

No problem, I'll get that posted shortly.

> 2) The TIF_NOTIFY_RESUME change is a nice cleanup on it's own and can go
>    before rc1 as well.

Would you mind taking that one? It's good to go.

> 3) Core infrastructure (patch 2 + 3 + 5) of this series
> 
>    Please make the changes I asked for in the generic entry code and
>    moving the handling into get_signal() for everybody else.
> 
>    So get_signal() gains:
> 
>      if (!IS_ENABLED(CONFIG_GENERIC_ENTRY) {
> 	 (test_thread_flag(TIF_NOTIFY_SIGNAL))
> 		tracehook_notify_signal();
> 
>          if (!task_sigpending(current))
>  		return 0;
>      }
> 
>    And with that you don't have to touch do_signal() in any architecture
>    except x86 which becomes:
> 
>    arch_do_signal_or_restart(bool sigpending)

Already did most of this, just need to handle the !CONFIG_GENERIC_ENTRY
for get_signal() and adapt the existing non-generic arch patches to
this.

> 4) Conversion of all architectures which means adding the TIF bit.
> 
>    If the architecture folks are happy, then this can be collected in
>    tip, which would be preferred because then 

Mostly done,
> 
> 5) Cleanups
> 
>    can just go on top.
> 
> Hmm?

Sounds good to me, as long as we keep the existing ordering with
x86/generic TIF_NOTIFY_SIGNAL support being able to move forward
before all archs have acked the arch specific change. Doesn't really
change how I'll get it done, and we're mostly there. Just don't
anything gated on potential slowest common denominator.

-- 
Jens Axboe