Skip to content

Commit 85253ba

Browse files
Christoph Hellwigaxboe
Christoph Hellwig
authored and
axboe
committed
block: don't free submitter owned integrity payload on I/O completion
Currently __bio_integrity_endio frees the integrity payload unless it is explicitly marked as user-mapped. This means in-kernel callers that allocate their own integrity payload never get to see it on I/O completion. The current two users don't need it as they just pre-mapped PI tuples received over the network, but this limits uses of integrity data lot. Change bio_integrity_endio to call __bio_integrity_endio for block layer generated integrity data only, and leave freeing of submitter allocated integrity data to bio_uninit which also gets called from the final bio_put. This requires that unmapping user mapped or copied integrity data is now always done by the caller, and the special BIP_INTEGRITY_USER flag can go away. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> Link: https://lore.kernel.org/r/20240702151047.1746127-6-hch@lst.de Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent f892437 commit 85253ba

File tree

3 files changed

+34
-39
lines changed

3 files changed

+34
-39
lines changed

‎block/bio-integrity.c‎

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,17 @@ void blk_flush_integrity(void)
2222
flush_workqueue(kintegrityd_wq);
2323
}
2424

25-
static void __bio_integrity_free(struct bio_set *bs,
26-
struct bio_integrity_payload *bip)
25+
/**
26+
* bio_integrity_free - Free bio integrity payload
27+
* @bio: bio containing bip to be freed
28+
*
29+
* Description: Free the integrity portion of a bio.
30+
*/
31+
void bio_integrity_free(struct bio *bio)
2732
{
33+
struct bio_integrity_payload *bip = bio_integrity(bio);
34+
struct bio_set *bs = bio->bi_pool;
35+
2836
if (bs && mempool_initialized(&bs->bio_integrity_pool)) {
2937
if (bip->bip_vec)
3038
bvec_free(&bs->bvec_integrity_pool, bip->bip_vec,
@@ -33,6 +41,8 @@ static void __bio_integrity_free(struct bio_set *bs,
3341
} else {
3442
kfree(bip);
3543
}
44+
bio->bi_integrity = NULL;
45+
bio->bi_opf &= ~REQ_INTEGRITY;
3646
}
3747

3848
/**
@@ -86,7 +96,10 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio *bio,
8696

8797
return bip;
8898
err:
89-
__bio_integrity_free(bs, bip);
99+
if (bs && mempool_initialized(&bs->bio_integrity_pool))
100+
mempool_free(bip, &bs->bio_integrity_pool);
101+
else
102+
kfree(bip);
90103
return ERR_PTR(-ENOMEM);
91104
}
92105
EXPORT_SYMBOL(bio_integrity_alloc);
@@ -132,28 +145,6 @@ static void bio_integrity_unmap_user(struct bio_integrity_payload *bip)
132145
bio_integrity_unpin_bvec(bip->bip_vec, bip->bip_max_vcnt, dirty);
133146
}
134147

135-
/**
136-
* bio_integrity_free - Free bio integrity payload
137-
* @bio: bio containing bip to be freed
138-
*
139-
* Description: Used to free the integrity portion of a bio. Usually
140-
* called from bio_free().
141-
*/
142-
void bio_integrity_free(struct bio *bio)
143-
{
144-
struct bio_integrity_payload *bip = bio_integrity(bio);
145-
struct bio_set *bs = bio->bi_pool;
146-
147-
if (bip->bip_flags & BIP_INTEGRITY_USER)
148-
return;
149-
if (bip->bip_flags & BIP_BLOCK_INTEGRITY)
150-
kfree(bvec_virt(bip->bip_vec));
151-
152-
__bio_integrity_free(bs, bip);
153-
bio->bi_integrity = NULL;
154-
bio->bi_opf &= ~REQ_INTEGRITY;
155-
}
156-
157148
/**
158149
* bio_integrity_unmap_free_user - Unmap and free bio user integrity payload
159150
* @bio: bio containing bip to be unmapped and freed
@@ -165,14 +156,9 @@ void bio_integrity_free(struct bio *bio)
165156
void bio_integrity_unmap_free_user(struct bio *bio)
166157
{
167158
struct bio_integrity_payload *bip = bio_integrity(bio);
168-
struct bio_set *bs = bio->bi_pool;
169159

170-
if (WARN_ON_ONCE(!(bip->bip_flags & BIP_INTEGRITY_USER)))
171-
return;
172160
bio_integrity_unmap_user(bip);
173-
__bio_integrity_free(bs, bip);
174-
bio->bi_integrity = NULL;
175-
bio->bi_opf &= ~REQ_INTEGRITY;
161+
bio_integrity_free(bio);
176162
}
177163

178164
/**
@@ -273,7 +259,7 @@ static int bio_integrity_copy_user(struct bio *bio, struct bio_vec *bvec,
273259
goto free_bip;
274260
}
275261

276-
bip->bip_flags |= BIP_INTEGRITY_USER | BIP_COPY_USER;
262+
bip->bip_flags |= BIP_COPY_USER;
277263
bip->bip_iter.bi_sector = seed;
278264
bip->bip_vcnt = nr_vecs;
279265
return 0;
@@ -294,7 +280,6 @@ static int bio_integrity_init_user(struct bio *bio, struct bio_vec *bvec,
294280
return PTR_ERR(bip);
295281

296282
memcpy(bip->bip_vec, bvec, nr_vecs * sizeof(*bvec));
297-
bip->bip_flags |= BIP_INTEGRITY_USER;
298283
bip->bip_iter.bi_sector = seed;
299284
bip->bip_iter.bi_size = len;
300285
bip->bip_vcnt = nr_vecs;
@@ -502,6 +487,8 @@ static void bio_integrity_verify_fn(struct work_struct *work)
502487
struct bio *bio = bip->bip_bio;
503488

504489
blk_integrity_verify(bio);
490+
491+
kfree(bvec_virt(bip->bip_vec));
505492
bio_integrity_free(bio);
506493
bio_endio(bio);
507494
}
@@ -522,13 +509,13 @@ bool __bio_integrity_endio(struct bio *bio)
522509
struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
523510
struct bio_integrity_payload *bip = bio_integrity(bio);
524511

525-
if (bio_op(bio) == REQ_OP_READ && !bio->bi_status &&
526-
(bip->bip_flags & BIP_BLOCK_INTEGRITY) && bi->csum_type) {
512+
if (bio_op(bio) == REQ_OP_READ && !bio->bi_status && bi->csum_type) {
527513
INIT_WORK(&bip->bip_work, bio_integrity_verify_fn);
528514
queue_work(kintegrityd_wq, &bip->bip_work);
529515
return false;
530516
}
531517

518+
kfree(bvec_virt(bip->bip_vec));
532519
bio_integrity_free(bio);
533520
return true;
534521
}

‎block/blk.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,20 @@ static inline unsigned int blk_queue_get_max_sectors(struct request *rq)
202202

203203
#ifdef CONFIG_BLK_DEV_INTEGRITY
204204
void blk_flush_integrity(void);
205-
bool __bio_integrity_endio(struct bio *);
206205
void bio_integrity_free(struct bio *bio);
206+
207+
/*
208+
* Integrity payloads can either be owned by the submitter, in which case
209+
* bio_uninit will free them, or owned and generated by the block layer,
210+
* in which case we'll verify them here (for reads) and free them before
211+
* the bio is handed back to the submitted.
212+
*/
213+
bool __bio_integrity_endio(struct bio *bio);
207214
static inline bool bio_integrity_endio(struct bio *bio)
208215
{
209-
if (bio_integrity(bio))
216+
struct bio_integrity_payload *bip = bio_integrity(bio);
217+
218+
if (bip && (bip->bip_flags & BIP_BLOCK_INTEGRITY))
210219
return __bio_integrity_endio(bio);
211220
return true;
212221
}

‎include/linux/bio-integrity.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ enum bip_flags {
1010
BIP_CTRL_NOCHECK = 1 << 2, /* disable HBA integrity checking */
1111
BIP_DISK_NOCHECK = 1 << 3, /* disable disk integrity checking */
1212
BIP_IP_CHECKSUM = 1 << 4, /* IP checksum */
13-
BIP_INTEGRITY_USER = 1 << 5, /* Integrity payload is user address */
14-
BIP_COPY_USER = 1 << 6, /* Kernel bounce buffer in use */
13+
BIP_COPY_USER = 1 << 5, /* Kernel bounce buffer in use */
1514
};
1615

1716
struct bio_integrity_payload {

0 commit comments

Comments
 (0)