On Tue 16-02-21 18:16:09, Jan Kara wrote: > On Tue 16-02-21 16:36:06, Christoph Hellwig wrote: > > On Tue, Feb 16, 2021 at 02:38:49PM +0100, Jan Kara wrote: > > > Apparently there are several userspace programs that depend on being > > > able to call BLKDISCARD ioctl without the ability to grab bdev > > > exclusively - namely FUSE filesystems have the device open without > > > O_EXCL (the kernel has the bdev open with O_EXCL) so the commit breaks > > > fstrim(8) for such filesystems. Also LVM when shrinking LV opens PV and > > > discards ranges released from LV but that PV may be already open > > > exclusively by someone else (see bugzilla link below for more details). > > > > > > This reverts commit 384d87ef2c954fc58e6c5fd8253e4a1984f5fe02. > > > > I think that is a bad idea. We fixed the problem for a reason. > > I think the right fix is to just do nothing if the device hasn't been > > opened with O_EXCL and can't be reopened with it, just don't do anything > > but also don't return an error. After all discard and thus > > BLKDISCARD is purely advisory. > > Yeah, certainly we'd have to fix the original problem in some other way. > Just silently ignoring BLKDISCARD if we cannot claim the device exclusively > is certainly an option to stop complaints from userspace. But note that > fstrim with fuse-based filesystem would still stay silent NOP which is > suboptimal. It could be fixed on FUSE side as I talked to Miklos but it > is not trivial. Similarly for the LVM regression... > > I was wondering whether we could do something like: > use truncate_inode_pages() if we can claim bdev exclusively > use invalidate_inode_pages2_range() if we cannot claim bdev > exclusively, possibly do nothing if that returns EBUSY? > > The downside is that cases where we cannot claim bdev exclusively would > unnecessarily write dirty buffer cache before discard. OK, no more comments I guess so I'll post this in a form of a patch and we'll see what people think. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
