| Message ID | e41b3b8e-16c2-70cb-97cb-881234bb200d@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | bio-integrity: don't restrict the size of integrity metadata | expand |
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
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.
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>
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,
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;