* [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-10-30 14:44 make block layer auto-PI deadlock safe v2 Christoph Hellwig
@ 2025-10-30 14:44 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-10-30 14:44 UTC (permalink / raw)
To: Jens Axboe; +Cc: Martin K. Petersen, linux-block
So remove the error check for it in bio_integrity_prep.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
block/bio-integrity-auto.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
index 687952f63bbb..2f4a244749ac 100644
--- a/block/bio-integrity-auto.c
+++ b/block/bio-integrity-auto.c
@@ -158,8 +158,6 @@ bool bio_integrity_prep(struct bio *bio)
if (!buf)
goto err_end_io;
bid = mempool_alloc(&bid_pool, GFP_NOIO);
- if (!bid)
- goto err_free_buf;
bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
bid->bio = bio;
@@ -187,8 +185,6 @@ bool bio_integrity_prep(struct bio *bio)
bid->saved_bio_iter = bio->bi_iter;
return true;
-err_free_buf:
- kfree(buf);
err_end_io:
bio->bi_status = BLK_STS_RESOURCE;
bio_endio(bio);
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* make block layer auto-PI deadlock safe v3
@ 2025-11-03 10:16 Christoph Hellwig
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-03 10:16 UTC (permalink / raw)
To: Jens Axboe; +Cc: Martin K. Petersen, linux-block
Hi all,
currently the automatic block layer PI generation allocates the integrity
buffer using kmalloc, and thus could deadlock, or fail I/O request due
to memory pressure.
Fix this by adding a mempool, and capping the maximum I/O size on PI
capable devices to not exceed the allocation size of the mempool.
This is against the block-6.18 branch as it has a contextual dependency
on the PI fix merged there yesterday.
Chances since v2:
- comment typo fixes
Chances since v1:
- keep the gfp flags manipulation local (at least for now)
- fix the maximum size calculation
Diffstat:
block/bio-integrity-auto.c | 26 ++--------------------
block/bio-integrity.c | 48 ++++++++++++++++++++++++++++++++++++++++++
block/blk-settings.c | 21 ++++++++++++++++++
include/linux/bio-integrity.h | 6 +++++
include/linux/blk-integrity.h | 5 ++++
5 files changed, 83 insertions(+), 23 deletions(-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 10:16 make block layer auto-PI deadlock safe v3 Christoph Hellwig
@ 2025-11-03 10:16 ` Christoph Hellwig
2025-11-03 11:16 ` Kanchan Joshi
` (3 more replies)
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 4 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-03 10:16 UTC (permalink / raw)
To: Jens Axboe; +Cc: Martin K. Petersen, linux-block
So remove the error check for it in bio_integrity_prep.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
block/bio-integrity-auto.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
index 687952f63bbb..2f4a244749ac 100644
--- a/block/bio-integrity-auto.c
+++ b/block/bio-integrity-auto.c
@@ -158,8 +158,6 @@ bool bio_integrity_prep(struct bio *bio)
if (!buf)
goto err_end_io;
bid = mempool_alloc(&bid_pool, GFP_NOIO);
- if (!bid)
- goto err_free_buf;
bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
bid->bio = bio;
@@ -187,8 +185,6 @@ bool bio_integrity_prep(struct bio *bio)
bid->saved_bio_iter = bio->bi_iter;
return true;
-err_free_buf:
- kfree(buf);
err_end_io:
bio->bi_status = BLK_STS_RESOURCE;
bio_endio(bio);
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 10:16 make block layer auto-PI deadlock safe v3 Christoph Hellwig
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
@ 2025-11-03 10:16 ` Christoph Hellwig
2025-11-03 11:15 ` Kanchan Joshi
` (2 more replies)
2025-11-04 19:47 ` make block layer auto-PI deadlock safe v3 Jens Axboe
2025-11-05 15:06 ` Jens Axboe
3 siblings, 3 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-03 10:16 UTC (permalink / raw)
To: Jens Axboe; +Cc: Martin K. Petersen, linux-block
The current block layer automatic integrity protection allocates the
actual integrity buffer, which has three problems:
- because it happens at the bottom of the I/O stack and doesn't use a
mempool it can deadlock under load
- because the data size in a bio is almost unbounded when using lage
folios it can relatively easily exceed the maximum kmalloc size
- even when it does not exceed the maximum kmalloc size, it could
exceed the maximum segment size of the device
Fix this by limiting the I/O size so that we can allocate at least a
2MiB integrity buffer, i.e. 128MiB for 8 byte PI and 512 byte integrity
intervals, and create a mempool as a last resort for this maximum size,
mirroring the scheme used for bvecs. As a nice upside none of this
can fail now, so we remove the error handling and open code the
trivial addition of the bip vec.
The new allocation helpers sit outside of bio-integrity-auto.c because
I plan to reuse them for file system based PI in the near future.
Fixes: 7ba1ba12eeef ("block: Block layer data integrity support")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
block/bio-integrity-auto.c | 22 +++-------------
block/bio-integrity.c | 48 +++++++++++++++++++++++++++++++++++
block/blk-settings.c | 21 +++++++++++++++
include/linux/bio-integrity.h | 6 +++++
include/linux/blk-integrity.h | 5 ++++
5 files changed, 83 insertions(+), 19 deletions(-)
diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
index 2f4a244749ac..9850c338548d 100644
--- a/block/bio-integrity-auto.c
+++ b/block/bio-integrity-auto.c
@@ -29,7 +29,7 @@ static void bio_integrity_finish(struct bio_integrity_data *bid)
{
bid->bio->bi_integrity = NULL;
bid->bio->bi_opf &= ~REQ_INTEGRITY;
- kfree(bvec_virt(bid->bip.bip_vec));
+ bio_integrity_free_buf(&bid->bip);
mempool_free(bid, &bid_pool);
}
@@ -110,8 +110,6 @@ bool bio_integrity_prep(struct bio *bio)
struct bio_integrity_data *bid;
bool set_flags = true;
gfp_t gfp = GFP_NOIO;
- unsigned int len;
- void *buf;
if (!bi)
return true;
@@ -152,17 +150,12 @@ bool bio_integrity_prep(struct bio *bio)
if (WARN_ON_ONCE(bio_has_crypt_ctx(bio)))
return true;
- /* Allocate kernel buffer for protection data */
- len = bio_integrity_bytes(bi, bio_sectors(bio));
- buf = kmalloc(len, gfp);
- if (!buf)
- goto err_end_io;
bid = mempool_alloc(&bid_pool, GFP_NOIO);
bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
-
bid->bio = bio;
-
bid->bip.bip_flags |= BIP_BLOCK_INTEGRITY;
+ bio_integrity_alloc_buf(bio, gfp & __GFP_ZERO);
+
bip_set_seed(&bid->bip, bio->bi_iter.bi_sector);
if (set_flags) {
@@ -174,21 +167,12 @@ bool bio_integrity_prep(struct bio *bio)
bid->bip.bip_flags |= BIP_CHECK_REFTAG;
}
- if (bio_integrity_add_page(bio, virt_to_page(buf), len,
- offset_in_page(buf)) < len)
- goto err_end_io;
-
/* Auto-generate integrity metadata if this is a write */
if (bio_data_dir(bio) == WRITE && bip_should_check(&bid->bip))
blk_integrity_generate(bio);
else
bid->saved_bio_iter = bio->bi_iter;
return true;
-
-err_end_io:
- bio->bi_status = BLK_STS_RESOURCE;
- bio_endio(bio);
- return false;
}
EXPORT_SYMBOL(bio_integrity_prep);
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index bed26f1ec869..09eeaf6e74b8 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -14,6 +14,45 @@ struct bio_integrity_alloc {
struct bio_vec bvecs[];
};
+static mempool_t integrity_buf_pool;
+
+void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
+{
+ struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
+ struct bio_integrity_payload *bip = bio_integrity(bio);
+ unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
+ gfp_t gfp = GFP_NOIO | (zero_buffer ? __GFP_ZERO : 0);
+ void *buf;
+
+ buf = kmalloc(len, (gfp & ~__GFP_DIRECT_RECLAIM) |
+ __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN);
+ if (unlikely(!buf)) {
+ struct page *page;
+
+ page = mempool_alloc(&integrity_buf_pool, GFP_NOFS);
+ if (zero_buffer)
+ memset(page_address(page), 0, len);
+ bvec_set_page(&bip->bip_vec[0], page, len, 0);
+ bip->bip_flags |= BIP_MEMPOOL;
+ } else {
+ bvec_set_page(&bip->bip_vec[0], virt_to_page(buf), len,
+ offset_in_page(buf));
+ }
+
+ bip->bip_vcnt = 1;
+ bip->bip_iter.bi_size = len;
+}
+
+void bio_integrity_free_buf(struct bio_integrity_payload *bip)
+{
+ struct bio_vec *bv = &bip->bip_vec[0];
+
+ if (bip->bip_flags & BIP_MEMPOOL)
+ mempool_free(bv->bv_page, &integrity_buf_pool);
+ else
+ kfree(bvec_virt(bv));
+}
+
/**
* bio_integrity_free - Free bio integrity payload
* @bio: bio containing bip to be freed
@@ -438,3 +477,12 @@ int bio_integrity_clone(struct bio *bio, struct bio *bio_src,
return 0;
}
+
+static int __init bio_integrity_initfn(void)
+{
+ if (mempool_init_page_pool(&integrity_buf_pool, BIO_POOL_SIZE,
+ get_order(BLK_INTEGRITY_MAX_SIZE)))
+ panic("bio: can't create integrity buf pool\n");
+ return 0;
+}
+subsys_initcall(bio_integrity_initfn);
diff --git a/block/blk-settings.c b/block/blk-settings.c
index d74b13ec8e54..547b679c5dc1 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -123,6 +123,19 @@ static int blk_validate_zoned_limits(struct queue_limits *lim)
return 0;
}
+/*
+ * Maximum size of I/O that needs a block layer integrity buffer. Limited
+ * by the number of intervals for which we can fit the integrity buffer into
+ * the buffer size. Because the buffer is a single segment it is also limited
+ * by the maximum segment size.
+ */
+static inline unsigned int max_integrity_io_size(struct queue_limits *lim)
+{
+ return min_t(unsigned int, lim->max_segment_size,
+ (BLK_INTEGRITY_MAX_SIZE / lim->integrity.metadata_size) <<
+ lim->integrity.interval_exp);
+}
+
static int blk_validate_integrity_limits(struct queue_limits *lim)
{
struct blk_integrity *bi = &lim->integrity;
@@ -194,6 +207,14 @@ static int blk_validate_integrity_limits(struct queue_limits *lim)
(1U << bi->interval_exp) - 1);
}
+ /*
+ * The block layer automatically adds integrity data for bios that don't
+ * already have it. Limit the I/O size so that a single maximum size
+ * metadata segment can cover the integrity data for the entire I/O.
+ */
+ lim->max_sectors = min(lim->max_sectors,
+ max_integrity_io_size(lim) >> SECTOR_SHIFT);
+
return 0;
}
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 851254f36eb3..3d05296a5afe 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -14,6 +14,8 @@ enum bip_flags {
BIP_CHECK_REFTAG = 1 << 6, /* reftag check */
BIP_CHECK_APPTAG = 1 << 7, /* apptag check */
BIP_P2P_DMA = 1 << 8, /* using P2P address */
+
+ BIP_MEMPOOL = 1 << 15, /* buffer backed by mempool */
};
struct bio_integrity_payload {
@@ -140,4 +142,8 @@ static inline int bio_integrity_add_page(struct bio *bio, struct page *page,
return 0;
}
#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
+void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer);
+void bio_integrity_free_buf(struct bio_integrity_payload *bip);
+
#endif /* _LINUX_BIO_INTEGRITY_H */
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index b659373788f6..c2030fd8ba0a 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -8,6 +8,11 @@
struct request;
+/*
+ * Maximum contiguous integrity buffer allocation.
+ */
+#define BLK_INTEGRITY_MAX_SIZE SZ_2M
+
enum blk_integrity_flags {
BLK_INTEGRITY_NOVERIFY = 1 << 0,
BLK_INTEGRITY_NOGENERATE = 1 << 1,
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
@ 2025-11-03 11:15 ` Kanchan Joshi
2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:40 ` Johannes Thumshirn
2 siblings, 0 replies; 21+ messages in thread
From: Kanchan Joshi @ 2025-11-03 11:15 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe; +Cc: Martin K. Petersen, linux-block
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
@ 2025-11-03 11:16 ` Kanchan Joshi
2025-11-03 11:26 ` John Garry
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Kanchan Joshi @ 2025-11-03 11:16 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe; +Cc: Martin K. Petersen, linux-block
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
2025-11-03 11:16 ` Kanchan Joshi
@ 2025-11-03 11:26 ` John Garry
2025-11-03 11:52 ` Christoph Hellwig
2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:28 ` Johannes Thumshirn
3 siblings, 1 reply; 21+ messages in thread
From: John Garry @ 2025-11-03 11:26 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe; +Cc: Martin K. Petersen, linux-block
On 03/11/2025 10:16, Christoph Hellwig wrote:
> So remove the error check for it in bio_integrity_prep.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
> block/bio-integrity-auto.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
> index 687952f63bbb..2f4a244749ac 100644
> --- a/block/bio-integrity-auto.c
> +++ b/block/bio-integrity-auto.c
> @@ -158,8 +158,6 @@ bool bio_integrity_prep(struct bio *bio)
> if (!buf)
> goto err_end_io;
> bid = mempool_alloc(&bid_pool, GFP_NOIO);
> - if (!bid)
> - goto err_free_buf;
> bio_integrity_init(bio, &bid->bip, &bid->bvec, 1);
>
> bid->bio = bio;
> @@ -187,8 +185,6 @@ bool bio_integrity_prep(struct bio *bio)
> bid->saved_bio_iter = bio->bi_iter;
> return true;
>
> -err_free_buf:
> - kfree(buf);
Is the original error handling code correct?
I would assume that if the call to bio_integrity_add_page() fails (when
it returns < len), then we should free buf (which does not happen).
Anyway, I see that this code is being changed in the next patch...
> err_end_io:
> bio->bi_status = BLK_STS_RESOURCE;
> bio_endio(bio);
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 11:26 ` John Garry
@ 2025-11-03 11:52 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-03 11:52 UTC (permalink / raw)
To: John Garry; +Cc: Christoph Hellwig, Jens Axboe, Martin K. Petersen, linux-block
On Mon, Nov 03, 2025 at 11:26:56AM +0000, John Garry wrote:
> Is the original error handling code correct?
Does it matter given that it was never called and is removed now? :)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
2025-11-03 11:15 ` Kanchan Joshi
@ 2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:40 ` Johannes Thumshirn
2 siblings, 0 replies; 21+ messages in thread
From: Anuj gupta @ 2025-11-03 13:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Martin K. Petersen, linux-block
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
2025-11-03 11:16 ` Kanchan Joshi
2025-11-03 11:26 ` John Garry
@ 2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:28 ` Johannes Thumshirn
3 siblings, 0 replies; 21+ messages in thread
From: Anuj gupta @ 2025-11-03 13:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, Martin K. Petersen, linux-block
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] block: blocking mempool_alloc doesn't fail
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
` (2 preceding siblings ...)
2025-11-03 13:20 ` Anuj gupta
@ 2025-11-03 13:28 ` Johannes Thumshirn
3 siblings, 0 replies; 21+ messages in thread
From: Johannes Thumshirn @ 2025-11-03 13:28 UTC (permalink / raw)
To: hch, Jens Axboe; +Cc: Martin K. Petersen, linux-block@vger.kernel.org
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
2025-11-03 11:15 ` Kanchan Joshi
2025-11-03 13:20 ` Anuj gupta
@ 2025-11-03 13:40 ` Johannes Thumshirn
2025-11-03 13:45 ` hch
2 siblings, 1 reply; 21+ messages in thread
From: Johannes Thumshirn @ 2025-11-03 13:40 UTC (permalink / raw)
To: hch, Jens Axboe; +Cc: Martin K. Petersen, linux-block@vger.kernel.org
On 11/3/25 11:18 AM, Christoph Hellwig wrote:
> +void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
> +{
> + struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
> + struct bio_integrity_payload *bip = bio_integrity(bio);
> + unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
> + gfp_t gfp = GFP_NOIO | (zero_buffer ? __GFP_ZERO : 0);
> + void *buf;
> +
> + buf = kmalloc(len, (gfp & ~__GFP_DIRECT_RECLAIM) |
> + __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN);
Why can't we clear the flags when assigning gfp, or at least outside of
kmalloc()s parameter list?
> + if (unlikely(!buf)) {
> + struct page *page;
> +
> + page = mempool_alloc(&integrity_buf_pool, GFP_NOFS);
> + if (zero_buffer)
> + memset(page_address(page), 0, len);
> + bvec_set_page(&bip->bip_vec[0], page, len, 0);
> + bip->bip_flags |= BIP_MEMPOOL;
> + } else {
> + bvec_set_page(&bip->bip_vec[0], virt_to_page(buf), len,
> + offset_in_page(buf));
> + }
> +
> + bip->bip_vcnt = 1;
> + bip->bip_iter.bi_size = len;
gfp is unused for the rest of the function.
> diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
> index 851254f36eb3..3d05296a5afe 100644
> --- a/include/linux/bio-integrity.h
> +++ b/include/linux/bio-integrity.h
> @@ -14,6 +14,8 @@ enum bip_flags {
> BIP_CHECK_REFTAG = 1 << 6, /* reftag check */
> BIP_CHECK_APPTAG = 1 << 7, /* apptag check */
> BIP_P2P_DMA = 1 << 8, /* using P2P address */
> +
> + BIP_MEMPOOL = 1 << 15, /* buffer backed by mempool */
> };
>
Any specific reason for the hole?
Otherwise,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 13:40 ` Johannes Thumshirn
@ 2025-11-03 13:45 ` hch
2025-11-03 13:47 ` Johannes Thumshirn
0 siblings, 1 reply; 21+ messages in thread
From: hch @ 2025-11-03 13:45 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: hch, Jens Axboe, Martin K. Petersen, linux-block@vger.kernel.org
On Mon, Nov 03, 2025 at 01:40:10PM +0000, Johannes Thumshirn wrote:
> On 11/3/25 11:18 AM, Christoph Hellwig wrote:
> > +void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
> > +{
> > + struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
> > + struct bio_integrity_payload *bip = bio_integrity(bio);
> > + unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
> > + gfp_t gfp = GFP_NOIO | (zero_buffer ? __GFP_ZERO : 0);
> > + void *buf;
> > +
> > + buf = kmalloc(len, (gfp & ~__GFP_DIRECT_RECLAIM) |
> > + __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN);
>
>
> Why can't we clear the flags when assigning gfp, or at least outside of
> kmalloc()s parameter list?
We could, but what's the point?
> > +++ b/include/linux/bio-integrity.h
> > @@ -14,6 +14,8 @@ enum bip_flags {
> > BIP_CHECK_REFTAG = 1 << 6, /* reftag check */
> > BIP_CHECK_APPTAG = 1 << 7, /* apptag check */
> > BIP_P2P_DMA = 1 << 8, /* using P2P address */
> > +
> > + BIP_MEMPOOL = 1 << 15, /* buffer backed by mempool */
> > };
> >
>
> Any specific reason for the hole?
Because it's really just an internal flag. (So is BIP_P2P_DMA, but
that should go away this merge window).
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 13:45 ` hch
@ 2025-11-03 13:47 ` Johannes Thumshirn
2025-11-03 13:49 ` hch
0 siblings, 1 reply; 21+ messages in thread
From: Johannes Thumshirn @ 2025-11-03 13:47 UTC (permalink / raw)
To: hch; +Cc: Jens Axboe, Martin K. Petersen, linux-block@vger.kernel.org
On 11/3/25 2:45 PM, hch wrote:
> On Mon, Nov 03, 2025 at 01:40:10PM +0000, Johannes Thumshirn wrote:
>> On 11/3/25 11:18 AM, Christoph Hellwig wrote:
>>> +void bio_integrity_alloc_buf(struct bio *bio, bool zero_buffer)
>>> +{
>>> + struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
>>> + struct bio_integrity_payload *bip = bio_integrity(bio);
>>> + unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
>>> + gfp_t gfp = GFP_NOIO | (zero_buffer ? __GFP_ZERO : 0);
>>> + void *buf;
>>> +
>>> + buf = kmalloc(len, (gfp & ~__GFP_DIRECT_RECLAIM) |
>>> + __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN);
>>
>> Why can't we clear the flags when assigning gfp, or at least outside of
>> kmalloc()s parameter list?
> We could, but what's the point?
From a reader pov it's kind of annoying to it touched twice like this,
but I guess that's just my personal style preference here.
>>> +++ b/include/linux/bio-integrity.h
>>> @@ -14,6 +14,8 @@ enum bip_flags {
>>> BIP_CHECK_REFTAG = 1 << 6, /* reftag check */
>>> BIP_CHECK_APPTAG = 1 << 7, /* apptag check */
>>> BIP_P2P_DMA = 1 << 8, /* using P2P address */
>>> +
>>> + BIP_MEMPOOL = 1 << 15, /* buffer backed by mempool */
>>> };
>>>
>> Any specific reason for the hole?
> Because it's really just an internal flag. (So is BIP_P2P_DMA, but
> that should go away this merge window).
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] block: make bio auto-integrity deadlock safe
2025-11-03 13:47 ` Johannes Thumshirn
@ 2025-11-03 13:49 ` hch
0 siblings, 0 replies; 21+ messages in thread
From: hch @ 2025-11-03 13:49 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: hch, Jens Axboe, Martin K. Petersen, linux-block@vger.kernel.org
On Mon, Nov 03, 2025 at 01:47:34PM +0000, Johannes Thumshirn wrote:
> >> Why can't we clear the flags when assigning gfp, or at least outside of
> >> kmalloc()s parameter list?
> > We could, but what's the point?
>
> From a reader pov it's kind of annoying to it touched twice like this,
> but I guess that's just my personal style preference here.
Note that the manipulations should eventually disappear in a common
mm-level helper as in v1. I'm just deferring this to get the fix in
without a cross-subsystem dependency that causes discussion on the
detailed approach.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-03 10:16 make block layer auto-PI deadlock safe v3 Christoph Hellwig
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
@ 2025-11-04 19:47 ` Jens Axboe
2025-11-05 13:23 ` Christoph Hellwig
2025-11-05 15:06 ` Jens Axboe
3 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2025-11-04 19:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-block
On 11/3/25 3:16 AM, Christoph Hellwig wrote:
> Hi all,
>
> currently the automatic block layer PI generation allocates the integrity
> buffer using kmalloc, and thus could deadlock, or fail I/O request due
> to memory pressure.
>
> Fix this by adding a mempool, and capping the maximum I/O size on PI
> capable devices to not exceed the allocation size of the mempool.
>
> This is against the block-6.18 branch as it has a contextual dependency
> on the PI fix merged there yesterday.
Yesterday? Guessing this got copy/pasted from the v1 posting, as it must
be this one:
commit 4c8cf6bd28d6fea23819f082ddc8063fd6fa963a (tag: block-6.18-20251023)
Author: Christoph Hellwig <hch@lst.de>
Date: Wed Oct 22 10:33:31 2025 +0200
block: require LBA dma_alignment when using PI
which is a bit longer ago.
But why? Surely this is 6.19 material?
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-04 19:47 ` make block layer auto-PI deadlock safe v3 Jens Axboe
@ 2025-11-05 13:23 ` Christoph Hellwig
2025-11-05 15:03 ` Jens Axboe
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-05 13:23 UTC (permalink / raw)
To: Jens Axboe; +Cc: Christoph Hellwig, Martin K. Petersen, linux-block
On Tue, Nov 04, 2025 at 12:47:09PM -0700, Jens Axboe wrote:
> > Fix this by adding a mempool, and capping the maximum I/O size on PI
> > capable devices to not exceed the allocation size of the mempool.
> >
> > This is against the block-6.18 branch as it has a contextual dependency
> > on the PI fix merged there yesterday.
>
> Yesterday? Guessing this got copy/pasted from the v1 posting, as it must
> be this one:
Yesterday as of the first post of the series, I should have updated this,
sorry.
>
> commit 4c8cf6bd28d6fea23819f082ddc8063fd6fa963a (tag: block-6.18-20251023)
> Author: Christoph Hellwig <hch@lst.de>
> Date: Wed Oct 22 10:33:31 2025 +0200
>
> block: require LBA dma_alignment when using PI
>
> which is a bit longer ago.
>
> But why? Surely this is 6.19 material?
This is 6.19 material. That's just a heads up that you need a merge
(or rebase which I was hoping for as of the first round, but it's probably
too late now)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-05 13:23 ` Christoph Hellwig
@ 2025-11-05 15:03 ` Jens Axboe
2025-11-05 19:46 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2025-11-05 15:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-block
On 11/5/25 6:23 AM, Christoph Hellwig wrote:
>> commit 4c8cf6bd28d6fea23819f082ddc8063fd6fa963a (tag: block-6.18-20251023)
>> Author: Christoph Hellwig <hch@lst.de>
>> Date: Wed Oct 22 10:33:31 2025 +0200
>>
>> block: require LBA dma_alignment when using PI
>>
>> which is a bit longer ago.
>>
>> But why? Surely this is 6.19 material?
>
> This is 6.19 material. That's just a heads up that you need a merge
> (or rebase which I was hoping for as of the first round, but it's probably
> too late now)
But why not generate it against the 6.19 tree then? As-is, I have to
first resolve the conflict in that tree, then resolve it again when
merging to for-next. Not a huge issue, just don't quite follow the logic
behind generating it against block-6.18.
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-03 10:16 make block layer auto-PI deadlock safe v3 Christoph Hellwig
` (2 preceding siblings ...)
2025-11-04 19:47 ` make block layer auto-PI deadlock safe v3 Jens Axboe
@ 2025-11-05 15:06 ` Jens Axboe
3 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2025-11-05 15:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-block
On Mon, 03 Nov 2025 05:16:43 -0500, Christoph Hellwig wrote:
> currently the automatic block layer PI generation allocates the integrity
> buffer using kmalloc, and thus could deadlock, or fail I/O request due
> to memory pressure.
>
> Fix this by adding a mempool, and capping the maximum I/O size on PI
> capable devices to not exceed the allocation size of the mempool.
>
> [...]
Applied, thanks!
[1/2] block: blocking mempool_alloc doesn't fail
commit: eef09f742be2a89126742f9f6f6a0d5d7c83cba8
[2/2] block: make bio auto-integrity deadlock safe
commit: ec7f31b2a2d3bf6b9e4d4b8cd156587f1d0607d5
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-05 15:03 ` Jens Axboe
@ 2025-11-05 19:46 ` Christoph Hellwig
2025-11-05 19:48 ` Jens Axboe
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2025-11-05 19:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: Christoph Hellwig, Martin K. Petersen, linux-block
On Wed, Nov 05, 2025 at 08:03:57AM -0700, Jens Axboe wrote:
> > This is 6.19 material. That's just a heads up that you need a merge
> > (or rebase which I was hoping for as of the first round, but it's probably
> > too late now)
>
> But why not generate it against the 6.19 tree then? As-is, I have to
> first resolve the conflict in that tree, then resolve it again when
> merging to for-next. Not a huge issue, just don't quite follow the logic
> behind generating it against block-6.18.
Mostly because I though you'd rebase when I did the first version and
then didn't really think about the base more after that.
Btw, we'll need the merge anyway for Keith's unaligned PI patch if we
end up adding that for 6.19.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: make block layer auto-PI deadlock safe v3
2025-11-05 19:46 ` Christoph Hellwig
@ 2025-11-05 19:48 ` Jens Axboe
0 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2025-11-05 19:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Martin K. Petersen, linux-block
On 11/5/25 12:46 PM, Christoph Hellwig wrote:
> On Wed, Nov 05, 2025 at 08:03:57AM -0700, Jens Axboe wrote:
>>> This is 6.19 material. That's just a heads up that you need a merge
>>> (or rebase which I was hoping for as of the first round, but it's probably
>>> too late now)
>>
>> But why not generate it against the 6.19 tree then? As-is, I have to
>> first resolve the conflict in that tree, then resolve it again when
>> merging to for-next. Not a huge issue, just don't quite follow the logic
>> behind generating it against block-6.18.
>
> Mostly because I though you'd rebase when I did the first version and
> then didn't really think about the base more after that.
Gotcha, yeah would've perhaps made sense back then, but way too late at
this point.
> Btw, we'll need the merge anyway for Keith's unaligned PI patch if we
> end up adding that for 6.19.
Yep did notice that. But since we already have a conflict, not big deal.
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-11-05 19:48 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03 10:16 make block layer auto-PI deadlock safe v3 Christoph Hellwig
2025-11-03 10:16 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
2025-11-03 11:16 ` Kanchan Joshi
2025-11-03 11:26 ` John Garry
2025-11-03 11:52 ` Christoph Hellwig
2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:28 ` Johannes Thumshirn
2025-11-03 10:16 ` [PATCH 2/2] block: make bio auto-integrity deadlock safe Christoph Hellwig
2025-11-03 11:15 ` Kanchan Joshi
2025-11-03 13:20 ` Anuj gupta
2025-11-03 13:40 ` Johannes Thumshirn
2025-11-03 13:45 ` hch
2025-11-03 13:47 ` Johannes Thumshirn
2025-11-03 13:49 ` hch
2025-11-04 19:47 ` make block layer auto-PI deadlock safe v3 Jens Axboe
2025-11-05 13:23 ` Christoph Hellwig
2025-11-05 15:03 ` Jens Axboe
2025-11-05 19:46 ` Christoph Hellwig
2025-11-05 19:48 ` Jens Axboe
2025-11-05 15:06 ` Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2025-10-30 14:44 make block layer auto-PI deadlock safe v2 Christoph Hellwig
2025-10-30 14:44 ` [PATCH 1/2] block: blocking mempool_alloc doesn't fail Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox