[PATCH] bio-integrity: don't restrict the size of integrity metadata

[PATCH] bio-integrity: don't restrict the size of integrity metadata

[Mikulas Patocka]

Hi Jens

I added dm-integrity inline mode in the 6.11 merge window. I've found out
that it doesn't work with large bios - the reason is that the function
bio_integrity_add_page refuses to add more metadata than
queue_max_hw_sectors(q). This restriction is no longer needed, because
big bios are split automatically. I'd like to ask you if you could send
this commit to Linus before 6.11 comes out, so that the bug is fixed
before the final release.

Mikulas


From: Mikulas Patocka <mpatocka@redhat.com>

bio_integrity_add_page restricts the size of the integrity metadata to
queue_max_hw_sectors(q). This restriction is not needed because oversized
bios are split automatically. This restriction causes problems with
dm-integrity 'inline' mode - if we send a large bio to dm-integrity and
the bio's metadata are larger than queue_max_hw_sectors(q),
bio_integrity_add_page fails and the bio is ended with BLK_STS_RESOURCE
error.

An example that triggers it:

# modprobe brd rd_size=1048576
# dmsetup create in1 --table '0 1847320 integrity /dev/ram0 0 64 D 1 fix_padding'
# dmsetup create in2 --table '0 1847312 integrity /dev/mapper/in1 0 64 I 1 internal_hash:sha512'
# dd if=/dev/zero of=/dev/mapper/in2 bs=1M oflag=direct status=progress
dd: error writing '/dev/mapper/in2': Cannot allocate memory
1+0 records in
0+0 records out
0 bytes copied, 0.00169291 s, 0.0 kB/s

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: fb0987682c62 ("dm-integrity: introduce the Inline mode")
Fixes: 0ece1d649b6d ("bio-integrity: create multi-page bvecs in bio_integrity_add_page()")

---
 block/bio-integrity.c |    4 ----
 1 file changed, 4 deletions(-)

Index: linux-2.6/block/bio-integrity.c
===================================================================
--- linux-2.6.orig/block/bio-integrity.c	2024-07-30 14:06:55.000000000 +0200
+++ linux-2.6/block/bio-integrity.c	2024-09-03 15:49:49.000000000 +0200
@@ -167,10 +167,6 @@ int bio_integrity_add_page(struct bio *b
 	struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 	struct bio_integrity_payload *bip = bio_integrity(bio);
 
-	if (((bip->bip_iter.bi_size + len) >> SECTOR_SHIFT) >
-	    queue_max_hw_sectors(q))
-		return 0;
-
 	if (bip->bip_vcnt > 0) {
 		struct bio_vec *bv = &bip->bip_vec[bip->bip_vcnt - 1];
 		bool same_page = false;

Re: [PATCH] bio-integrity: don't restrict the size of integrity metadata

[Ming Lei]

On Tue, Sep 03, 2024 at 09:47:59PM +0200, Mikulas Patocka wrote:
> Hi Jens
> 
> I added dm-integrity inline mode in the 6.11 merge window. I've found out
> that it doesn't work with large bios - the reason is that the function
> bio_integrity_add_page refuses to add more metadata than
> queue_max_hw_sectors(q). This restriction is no longer needed, because
> big bios are split automatically. I'd like to ask you if you could send
> this commit to Linus before 6.11 comes out, so that the bug is fixed
> before the final release.
> 
> Mikulas
> 
> 
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> bio_integrity_add_page restricts the size of the integrity metadata to
> queue_max_hw_sectors(q). This restriction is not needed because oversized
> bios are split automatically. This restriction causes problems with
> dm-integrity 'inline' mode - if we send a large bio to dm-integrity and
> the bio's metadata are larger than queue_max_hw_sectors(q),
> bio_integrity_add_page fails and the bio is ended with BLK_STS_RESOURCE
> error.
> 
> An example that triggers it:
> 
> # modprobe brd rd_size=1048576
> # dmsetup create in1 --table '0 1847320 integrity /dev/ram0 0 64 D 1 fix_padding'
> # dmsetup create in2 --table '0 1847312 integrity /dev/mapper/in1 0 64 I 1 internal_hash:sha512'
> # dd if=/dev/zero of=/dev/mapper/in2 bs=1M oflag=direct status=progress
> dd: error writing '/dev/mapper/in2': Cannot allocate memory
> 1+0 records in
> 0+0 records out
> 0 bytes copied, 0.00169291 s, 0.0 kB/s
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Fixes: fb0987682c62 ("dm-integrity: introduce the Inline mode")
> Fixes: 0ece1d649b6d ("bio-integrity: create multi-page bvecs in bio_integrity_add_page()")

Firstly meta size is always < bio size.

Secondly the check isn't needed either bio_integrity_add_page() is called on
to-be-splited bio(DM), or splited bio(blk-mq).

Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks,
Ming

Re: [PATCH] bio-integrity: don't restrict the size of integrity metadata

[Christoph Hellwig]

On Tue, Sep 03, 2024 at 09:47:59PM +0200, Mikulas Patocka wrote:
> Hi Jens
> 
> I added dm-integrity inline mode in the 6.11 merge window. I've found out
> that it doesn't work with large bios - the reason is that the function
> bio_integrity_add_page refuses to add more metadata than
> queue_max_hw_sectors(q). This restriction is no longer needed, because
> big bios are split automatically. I'd like to ask you if you could send
> this commit to Linus before 6.11 comes out, so that the bug is fixed
> before the final release.

This looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Although between the patches from Keith and the prep patch for the
io_uring metadata series I'm not sure that splitting of bios with
integrity actually fully works.  Although until we get the io_uring
passthrough code in we also won't have an easy way to exercise it either.

Re: [PATCH] bio-integrity: don't restrict the size of integrity metadata

[Anuj gupta]

On Wed, Sep 4, 2024 at 1:18 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> Hi Jens
>
> I added dm-integrity inline mode in the 6.11 merge window. I've found out
> that it doesn't work with large bios - the reason is that the function
> bio_integrity_add_page refuses to add more metadata than
> queue_max_hw_sectors(q). This restriction is no longer needed, because
> big bios are split automatically. I'd like to ask you if you could send
> this commit to Linus before 6.11 comes out, so that the bug is fixed
> before the final release.

Tested-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH] bio-integrity: don't restrict the size of integrity metadata

[Jens Axboe]

On Tue, 03 Sep 2024 21:47:59 +0200, Mikulas Patocka wrote:
> I added dm-integrity inline mode in the 6.11 merge window. I've found out
> that it doesn't work with large bios - the reason is that the function
> bio_integrity_add_page refuses to add more metadata than
> queue_max_hw_sectors(q). This restriction is no longer needed, because
> big bios are split automatically. I'd like to ask you if you could send
> this commit to Linus before 6.11 comes out, so that the bug is fixed
> before the final release.
> 
> [...]

Applied, thanks!

[1/1] bio-integrity: don't restrict the size of integrity metadata
      commit: b858a36fe9a1261dfd097aec855161ad135bed60

Best regards,
-- 
Jens Axboe