Skip to contentSkip to conversation

block: silently error unsupported empty barriers too

PATCH In review Mark McLoughlin 7 participants 13 messages Aug 6, 2009 – Oct 29, 2009

Subsystems (3)
Block layer virtio core Device mapper
lore
Share & tools
lore

Mark McLoughlin

author
#0 lore raw

PATCH 1/1 block: silently error unsupported empty barriers too

With 2.6.31-rc5 in a KVM guest using dm and virtio_blk, we see the following errors:

end_request: I/O error, dev vda, sector 0
end_request: I/O error, dev vda, sector 0

The errors go away if dm stops submitting empty barriers, by reverting:

commit 52b1fd5a27c625c78373e024bf570af3c9d44a79
Author: Mikulas Patocka <[email protected]>
dm: send empty barriers to targets in dm_flush

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

See also:

https://bugzilla.redhat.com/514901

Signed-off-by Mark McLoughlin <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Mikulas Patocka <[email protected]>
Cc Alasdair G Kergon <[email protected]>
Cc Neil Brown <[email protected]>
---
 block/blk-core.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
block/blk-core.c +1 -2
index e3299a7..35ad2bb 100644
@@ -1163,8 +1163,7 @@ static int __make_request(struct request_queue *q, struct bio *bio)
 	const int unplug = bio_unplug(bio);
 	int rw_flags;
 
-	if (bio_barrier(bio) && bio_has_data(bio) &&
-	    (q->next_ordered == QUEUE_ORDERED_NONE)) {
+	if (bio_barrier(bio) && (q->next_ordered == QUEUE_ORDERED_NONE)) {
 		bio_endio(bio, -EOPNOTSUPP);
 		return 0;
 	}
Signature · 1 line
1.6.4
maintainer
#1 lore raw
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

#2 lore raw
Quoted 6 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

virtio_blk on kvm does not support any barriers at all, similar to many other drivers out there. If the queue flags say we don't support barriers higher layers should not submit any barrier requests.

#3 lore raw
In reply to Alasdair G Kergon
Quoted 6 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

virtio_blk on kvm does not support any barriers at all, similar to many other drivers out there. If the queue flags say we don't support barriers higher layers should not submit any barrier requests.

maintainer
#4 lore raw
In reply to Alasdair G Kergon
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas

#5 lore raw
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

Andrew

Signature · 2 lines
Andrew Patterson
Hewlett-Packard
#6 lore raw
Quoted 30 lines · Andrew Patterson
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

Quoted 8 lines

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

It was resubmitted last Friday:
http://lkml.org/lkml/2009/10/23/196

And is queued in Jens' 'for-linus' branch (meaning it should get upstream in time for 2.6.32), see:
http://git.kernel.org/?p=linux/kernel/git/axboe/linux-2.6-block.git;a=commit;h=6cafb12dc85a5bdc722791cc5070968413264909

Mike

#7 lore raw
Quoted 42 lines · Mike Snitzer
Quoted 30 lines · Andrew Patterson
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

Quoted 8 lines

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

Quoted 6 lines

It was resubmitted last Friday:
http://lkml.org/lkml/2009/10/23/196

And is queued in Jens' 'for-linus' branch (meaning it should get upstream in time for 2.6.32), see:
http://git.kernel.org/?p=linux/kernel/git/axboe/linux-2.6-block.git;a=commit;h=6cafb12dc85a5bdc722791cc5070968413264909

I sent out a pull request yesterday, but it isn't in yet. Any day now...

Signature · 1 line
Jens Axboe
#8 lore raw
In reply to Mike Snitzer
Quoted 42 lines · Mike Snitzer
Quoted 30 lines · Andrew Patterson
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

Quoted 8 lines

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

Quoted 6 lines

It was resubmitted last Friday:
http://lkml.org/lkml/2009/10/23/196

And is queued in Jens' 'for-linus' branch (meaning it should get upstream in time for 2.6.32), see:
http://git.kernel.org/?p=linux/kernel/git/axboe/linux-2.6-block.git;a=commit;h=6cafb12dc85a5bdc722791cc5070968413264909

I sent out a pull request yesterday, but it isn't in yet. Any day now...

Signature · 1 line
Jens Axboe
#9 lore raw
In reply to Andrew Patterson
Quoted 30 lines · Andrew Patterson
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

Quoted 8 lines

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

It was resubmitted last Friday:
http://lkml.org/lkml/2009/10/23/196

And is queued in Jens' 'for-linus' branch (meaning it should get upstream in time for 2.6.32), see:
http://git.kernel.org/?p=linux/kernel/git/axboe/linux-2.6-block.git;a=commit;h=6cafb12dc85a5bdc722791cc5070968413264909

Mike

#10 lore raw
In reply to Mikulas Patocka
Quoted 11 lines · Mikulas Patocka
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

Quoted 7 lines

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas
--

This patch seems to have been dropped. I can reproduce the error using the following:

1. Create an LVM logical volume on top of a cciss device (note that the cciss driver does not support barriers)
2. Create an ext3 file system on top of the logical volume
3. Mount the file-system using -obarrier=1
4. Copy some files onto the file-system
5. Run sync (dm_flush is called)

If I apply the patch (with some munging), the

"end_request: I/O error, dev cciss/cXdY, sector 0"

messages go away.

This is not strictly a regression, given that the problem was introduced in 2.6.31, but I think it should still be fixed for 2.6.32.

Andrew

Signature · 2 lines
Andrew Patterson
Hewlett-Packard
maintainer
#11 lore raw
In reply to Alasdair G Kergon
Quoted 8 lines · Alasdair G Kergon
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair

This is only for request-based drivers, where it is the responsibility of blk-core to translate barriers. I think the empty barrier request anyway in blk_do_ordered, but with an error message. So the patch changes it to discard it early and queitly. It seems ok.

Mikulas

maintainer
#12 lore raw
In reply to Mark McLoughlin
Quoted 2 lines · Mark McLoughlin

We should error all barriers, even empty barriers, on devices like virtio_blk which don't support them.

Have you considered whether or not virtio_blk actually needs to support empty barriers?

Alasdair