[PATCH] block: flush the disk cache on BLKFLSBUF

[PATCH] block: flush the disk cache on BLKFLSBUF

[Mikulas Patocka]

The BLKFLSBUF ioctl doesn't send the flush bio to the block device, thus
flushed data may be lurking in the disk cache and they may not be really
flushed to the stable storage.

This patch adds the call to blkdev_issue_flush to blkdev_flushbuf.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 block/ioctl.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/block/ioctl.c
===================================================================
--- linux-2.6.orig/block/ioctl.c
+++ linux-2.6/block/ioctl.c
@@ -351,6 +351,7 @@ static int blkdev_flushbuf(struct block_
 		return -EACCES;
 	fsync_bdev(bdev);
 	invalidate_bdev(bdev);
+	blkdev_issue_flush(bdev);
 	return 0;
 }
 

Re: [PATCH] block: flush the disk cache on BLKFLSBUF

[Christoph Hellwig]

On Mon, Jun 26, 2023 at 10:25:28PM +0200, Mikulas Patocka wrote:
> The BLKFLSBUF ioctl doesn't send the flush bio to the block device, thus
> flushed data may be lurking in the disk cache and they may not be really
> flushed to the stable storage.
> 
> This patch adds the call to blkdev_issue_flush to blkdev_flushbuf.

Umm, why?  This is an ioctl no one should be using, and we certainly
should not add new functionality to it.  Can you explain what you're
trying to do here?

Re: [PATCH] block: flush the disk cache on BLKFLSBUF

[Mikulas Patocka]

> On Mon, 26 Jun 2023, Mikulas Patocka wrote:
> 
> > The BLKFLSBUF ioctl doesn't send the flush bio to the block device, thus
> > flushed data may be lurking in the disk cache and they may not be really
> > flushed to the stable storage.
> > 
> > This patch adds the call to blkdev_issue_flush to blkdev_flushbuf.
> 
> Umm, why?  This is an ioctl no one should be using, and we certainly
> should not add new functionality to it.  Can you explain what you're
> trying to do here?

Marc Smith reported a bug where he wrote to the dm-writecache target using 
O_DIRECT, then reset the machine without proper shutdown and the freshly 
written data were lost. It turned out that he didn't use the fsync or 
fdatasync syscall (and dm-writecache makes its metadata persistent on a 
FLUSH bio).

When I was analyzing this issue, it turned out that there is no easy way 
how to send the FLUSH bio to a block device from a command line.

The sync command synchronizes only filesystems, it doesn't flush cache for 
unmounted block devices (do you think that it should flush block devices 
too?).

The "blockdev --flushbufs" command also doesn't send the FLUSH bio, but I 
would expect it to send it. Without sending the FLUSH bio, "blockdev 
--flushbufs" doesn't really guarantee anything.

Mikulas

Re: [PATCH] block: flush the disk cache on BLKFLSBUF

[Christoph Hellwig]

On Tue, Jun 27, 2023 at 05:31:12PM +0200, Mikulas Patocka wrote:
> Marc Smith reported a bug where he wrote to the dm-writecache target using 
> O_DIRECT, then reset the machine without proper shutdown and the freshly 
> written data were lost. It turned out that he didn't use the fsync or 
> fdatasync syscall (and dm-writecache makes its metadata persistent on a 
> FLUSH bio).

Which so far is expected.  Even with O_DIRECT you need O_(D)SYNC or
fsync/fdatasync to persist data.

> When I was analyzing this issue, it turned out that there is no easy way 
> how to send the FLUSH bio to a block device from a command line.

xfs_io -c fsync /dev/foo

> The "blockdev --flushbufs" command also doesn't send the FLUSH bio, but I 
> would expect it to send it. Without sending the FLUSH bio, "blockdev 
> --flushbufs" doesn't really guarantee anything.

I wouldn't expect it.  It's a really weird legacy thing that calls
back up into the file system, but only if it sets s_bdev to this
device.  I don't think we should add new users of it that overload the
semantics.