All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>, Song Liu <songliubraving@fb.com>
Subject: Re: [PATCH] fs: process fput task_work with TWA_SIGNAL
Date: Fri, 8 Jan 2021 18:05:35 +0000	[thread overview]
Message-ID: <20210108180535.GR3579531@ZenIV.linux.org.uk> (raw)
In-Reply-To: <2cdd6d47-7eb1-3ab1-7aa8-80c54819009b@kernel.dk>

On Fri, Jan 08, 2021 at 09:26:40AM -0700, Jens Axboe wrote:
> >> Can you show the callers that DO NOT need it?
> > 
> > OK, so here's my suggestion:
> > 
> > 1) For 5.11, we just re-instate the task_work run in get_signal(). This
> >    will make TWA_RESUME have the exact same behavior as before.
> > 
> > 2) For 5.12, I'll prepare a patch that collapses TWA_RESUME and TWA_SIGNAL,
> >    turning it into a bool again (notify or no notify).
> > 
> > How does that sound?
> 
> Attached the patches - #1 is proposed for 5.11 to fix the current issue,
> and then 2-4 can get queued for 5.12 to totally remove the difference
> between TWA_RESUME and TWA_SIGNAL.
> 
> Totally untested, but pretty straight forward.

	Umm...  I'm looking at the callers of get_signal() and I really wonder
how your support for TIF_NOTIFY_SIGNAL interacts with saved sigmask handling
by various do_signal() (calls of restore_saved_sigmask()).  Could you give
pointers to relevant discussion or a braindump on the same?  I realize that
it had been months ago, but...

	Do we even need restore_saved_sigmask_unless() now?  Could
set_user_sigmask() simply set TIF_NOTIFY_SIGNAL?  Oleg, could you comment
on that?

	Another fun question is how does that thing interact with
single-stepping logics; it's been about 8 years since I looked into
those horrors, but they used to be bloody awful...

	What I'm trying to figure out is how costly TIF_NOTIFY_SIGNAL is
on the work execution side; task_work_add() side is cheap enough, it's
delivery that is interesting.

      reply	other threads:[~2021-01-08 18:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-05 18:29 [PATCH] fs: process fput task_work with TWA_SIGNAL Jens Axboe
2021-01-07 22:17 ` Doug Anderson
2021-01-08  3:52   ` Jens Axboe
2021-01-08  6:20     ` Sedat Dilek
2021-01-08  5:26 ` Al Viro
2021-01-08  6:21   ` Sedat Dilek
2021-01-08  6:47     ` Al Viro
2021-01-08  6:52     ` Al Viro
2021-01-08  6:46   ` Al Viro
2021-01-08 15:13     ` Jens Axboe
2021-01-08 15:58       ` Al Viro
2021-01-08 16:10         ` Jens Axboe
2021-01-08 16:26           ` Jens Axboe
2021-01-08 18:05             ` Al Viro [this message]

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=20210108180535.GR3579531@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=songliubraving@fb.com \
    /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.