On Thu, Mar 07, 2024 at 07:51:35AM +1100, Dave Chinner wrote: > On Wed, Mar 06, 2024 at 07:18:02AM -0800, Christoph Hellwig wrote: > > Lookings at this a bit more I'm not sure my fix is enough as the error > > handling is really complex. Also given that some discard callers are > > from kernel threads messing with interruptibility I'm not entirely > > sure that having this check in the common helper is a good idea. > > Yeah, this seems like a problem. The only places that userspace > should be issuing discards directly and hence be interruptible from > are FITRIM, BLKDISCARD and fallocate() on block devices. Yes. > Filesystems already handle fatal signals in FITRIM (e.g. see > xfs_trim_should_stop(), ext4_trim_interrupted(), > btrfs_trim_free_extents(), etc), so it seems to me that the only > non-interruptible call from userspace are operations directly on > block devices which have no higher level iteration over the range to > discard and the user controls the range directly. Yeah. > Perhaps the solution is to change BLKDISCARD/fallocate() on bdev to > look more like xfs_discard_extents() where it breaks the range up > into smaller chunks and intersperses bio chaining with signal > checks. Well, xfs_discard_extents has different extents from the higher layers. __blkdev_issue_discard than breaks it up based on what fits into the bio (and does some alignment against our normal rule of leaving that to the splitting code). But I suspect moving the loop in __blkdev_issue_discard into the callers could really help with this. > > I suspect the same solution is necessary for blkdev_issue_zeroout() > and blkdev_issue_secure_erase(), because both of them have user > controlled lengths... Yes. (or rather two sub cases of the former and the latter)
