Re: [PATCHv2] blk-lib: check for kill signal

[Keith Busch <kbusch@kernel.org>]

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.