diff mbox series

drbd: do not set REQ_PREFLUSH when submitting barrier

Message ID 20230502092922.175857-1-christoph.boehmwalder@linbit.com (mailing list archive)
State New, archived
Headers show
Series drbd: do not set REQ_PREFLUSH when submitting barrier | expand

Commit Message

Christoph Böhmwalder May 2, 2023, 9:29 a.m. UTC
When we receive a flush command (or "barrier" in DRBD), we currently use
a REQ_OP_FLUSH with the REQ_PREFLUSH flag set.

REQ_OP_FLUSH is supposed to be an empty bio with the sole purpose of
flushing the disk. REQ_PREFLUSH is used for REQ_OP_WRITE bios to
additionally signal that a flush should be issued, so it is redundant
here.

Since commit b4a6bb3a67aa ("block: add a sanity check for non-write
flush/fua bios"), this triggers a warning in the block layer.
So remove the REQ_PREFLUSH flag when allocating the bio.

Fixes: f9ff0da56437 ("drbd: allow parallel flushes for multi-volume resources")
Reported-by: Thomas Voegtle <tv@lio96.de>
Signed-off-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
---
 drivers/block/drbd/drbd_receiver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig May 3, 2023, 4:18 a.m. UTC | #1
On Tue, May 02, 2023 at 11:29:22AM +0200, Christoph Böhmwalder wrote:
>  	struct bio *bio = bio_alloc(device->ldev->backing_bdev, 0,
> -				    REQ_OP_FLUSH | REQ_PREFLUSH, GFP_NOIO);
> +				    REQ_OP_FLUSH, GFP_NOIO);

This isn't going to work.  flush bios are (somewhat confusingly)
implemented as writes with no data and the preflush flag.  So this
should be:

	REQ_OP_WRITE | REQ_PREFLUSH.

and it looks like whatever flushing this does hasn't wroked for a long
time.
Christoph Böhmwalder May 3, 2023, 11:05 a.m. UTC | #2
Am 03.05.23 um 06:18 schrieb Christoph Hellwig:
> On Tue, May 02, 2023 at 11:29:22AM +0200, Christoph Böhmwalder wrote:
>>  	struct bio *bio = bio_alloc(device->ldev->backing_bdev, 0,
>> -				    REQ_OP_FLUSH | REQ_PREFLUSH, GFP_NOIO);
>> +				    REQ_OP_FLUSH, GFP_NOIO);
> 
> This isn't going to work.  flush bios are (somewhat confusingly)

Indeed...

> implemented as writes with no data and the preflush flag.  So this
> should be:
> 
> 	REQ_OP_WRITE | REQ_PREFLUSH.
> 
> and it looks like whatever flushing this does hasn't wroked for a long
> time.

Thanks for the hint. I'll prepare a v2 today.
diff mbox series

Patch

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index e54404c632e7..f2479c29197a 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -1283,7 +1283,7 @@  static void one_flush_endio(struct bio *bio)
 static void submit_one_flush(struct drbd_device *device, struct issue_flush_context *ctx)
 {
 	struct bio *bio = bio_alloc(device->ldev->backing_bdev, 0,
-				    REQ_OP_FLUSH | REQ_PREFLUSH, GFP_NOIO);
+				    REQ_OP_FLUSH, GFP_NOIO);
 	struct one_flush_context *octx = kmalloc(sizeof(*octx), GFP_NOIO);
 
 	if (!octx) {