All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: io-uring <io-uring@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH RFC v2] kernel: decouple TASK_WORK TWA_SIGNAL handling from signals
Date: Thu, 1 Oct 2020 18:27:20 +0200	[thread overview]
Message-ID: <20201001162719.GD13633@redhat.com> (raw)
In-Reply-To: <3ce9e205-aad0-c9ce-86a7-b281f1c0237a@kernel.dk>

Jens,

I'll read this version tomorrow, but:

On 10/01, Jens Axboe wrote:
>
>  static inline int signal_pending(struct task_struct *p)
>  {
> -	return unlikely(test_tsk_thread_flag(p,TIF_SIGPENDING));
> +#ifdef TIF_TASKWORK
> +	/*
> +	 * TIF_TASKWORK isn't really a signal, but it requires the same
> +	 * behavior of restarting the system call to force a kernel/user
> +	 * transition.
> +	 */
> +	return unlikely(test_tsk_thread_flag(p, TIF_SIGPENDING) ||
> +			test_tsk_thread_flag(p, TIF_TASKWORK));
> +#else
> +	return unlikely(test_tsk_thread_flag(p, TIF_SIGPENDING));
> +#endif

This change alone is already very wrong.

signal_pending(task) == T means that this task will do get_signal() as
soon as it can, and this basically means you can't "divorce" SIGPENDING
and TASKWORK.

Simple example. Suppose we have a single-threaded task T.

Someone does task_work_add(T, TWA_SIGNAL). This makes signal_pending()==T
and this is what we need.

Now suppose that another task sends a signal to T before T calls
task_work_run() and clears TIF_TASKWORK. In this case SIGPENDING won't
be set because signal_pending() is already set (see wants_signal), and
this means that T won't notice this signal.

Oleg.


  reply	other threads:[~2020-10-01 16:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 15:03 [PATCH RFC v2] kernel: decouple TASK_WORK TWA_SIGNAL handling from signals Jens Axboe
2020-10-01 16:27 ` Oleg Nesterov [this message]
2020-10-01 17:27   ` Jens Axboe
     [not found]   ` <20201002133813.3180-1-hdanton@sina.com>
2020-10-02 13:44     ` Jens Axboe

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=20201001162719.GD13633@redhat.com \
    --to=oleg@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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.