On Thu, Feb 22, 2024 at 12:02:36PM +0800, Ming Lei wrote:
> On Wed, Feb 21, 2024 at 02:20:13PM -0800, Keith Busch wrote:
> > After the kill signal is observered, instead of submitting and waiting
> > for the current parent bio in the chain, abort it by ending it
> > immediately and do the final bio_put() after every previously submitted
> > chained bio completes.
>
> I feel this way is fragile:
>
> 1) user sends KILL signal
>
> 2) discard API returns
>
> 3) submitted discard requests are still run in background, and there
> can be thousands of such bios
>
> 4) what if application or FS code(such as meta) starts to write data to
> the discard range?
Right, there's no IO order guarantee there, and sounds reasonable to
expect no potential conflicts after the function returns. We could add a
similiar completion that submit_bio_wait() uses to ensure the previous
bio's are all done before returning. At least that looks safe to do for
any case where fatal signal would apply.
> > + if (fatal_signal_pending(current)) {
> > + abort_bio(bio);
> > + ret = -EINTR;
> > + bio = NULL;
> > + }
>
> The handling for blkdev_issue_secure_erase is different with others, and
> actually it doesn't return immediately, care to add comment?
Ha, I actually prepared a patch to make secure_erase look like everyone
else. I chose the smaller diff, but it does look weird. I'll reconsider
that for the next version.
