Re: [PATCH] Revert "block: Do not discard buffers under a mounted filesystem"

[Jan Kara <jack@suse.cz>]

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@suse.com>
SUSE Labs, CR