On 11/3/24 22:51, Jens Axboe wrote:
On 11/3/24 3:47 PM, Pavel Begunkov wrote:
On 11/3/24 22:40, Jens Axboe wrote:
...
Right, but:
if (current->flags & (PF_EXITING | PF_KTHREAD))
...
should be fine as it'll catch both cases with the single check.
Was thinking to mention it, it should be fine buf feels wrong. Instead
of directly checking what we want, i.e. whether the task we want to run
the request from is dead, we are now doing "let's check if the task
is dead. Ah yes, let's also see if it's PF_KTHREAD which indirectly
implies that the task is dead because of implementation details."
Should be fine to leave that, but why not just leave the check
how it was? Even if it now requires an extra deref through tctx.
I think it'd be better with a comment, I added one that says:
/* exiting original task or fallback work, cancel */
We can retain the original check, but it's actually a data race to check
->flags from a different task. Yes for this case we're in fallback work
and the value should be long since stable, but seems prudent to just
check for the two criteria we care about. At least the comment will be
correct now ;-)
I don't think whack-a-mole'ing all cases is a good thing,
but at least it can get moved into a helper and be reused in
all other places.
if (io_tw_should_terminate(req, tw))
fail;
should be more readable
--
Pavel Begunkov
