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 15:58:07 +0000 [thread overview]
Message-ID: <20210108155807.GQ3579531@ZenIV.linux.org.uk> (raw)
In-Reply-To: <245fba32-76cc-c4e1-6007-0b1f8a22a86b@kernel.dk>
On Fri, Jan 08, 2021 at 08:13:25AM -0700, Jens Axboe wrote:
> > Anyway, bedtime for me; right now it looks like at least for task ==
> > current we always want TWA_SIGNAL. I'll look into that more tomorrow
> > when I get up, but so far it smells like switching everything to
> > TWA_SIGNAL would be the right thing to do, if not going back to bool
> > notify for task_work_add()...
>
> Before the change, the fact that we ran task_work off get_signal() and
> thus processed even non-notify work in that path was a bit of a mess,
> imho. If you have work that needs processing now, in the same manner as
> signals, then you really should be using TWA_SIGNAL. For this pipe case,
> and I'd need to setup and reproduce it again, the task must have a
> signal pending and that would have previously caused the task_work to
> run, and now it does not. TWA_RESUME technically didn't change its
> behavior, it's still the same notification type, we just don't run
> task_work unconditionally (regardless of notification type) from
> get_signal().
It sure as hell did change behaviour. Think of the effect of getting
hit with SIGSTOP. That's what that "bit of a mess" had been about.
Work done now vs. possibly several days later when SIGCONT finally
gets sent.
> I think the main question here is if we want to re-instate the behavior
> of running task_work off get_signal(). I'm leaning towards not doing
> that and ensuring that callers that DO need that are using TWA_SIGNAL.
Can you show the callers that DO NOT need it?
next prev parent reply other threads:[~2021-01-08 15:59 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 [this message]
2021-01-08 16:10 ` Jens Axboe
2021-01-08 16:26 ` Jens Axboe
2021-01-08 18:05 ` Al Viro
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=20210108155807.GQ3579531@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.