On 6/28/2024 6:57 PM, Christoph Hellwig wrote:
Thanks for streamlining this. Few comments below.
> static inline bool bio_integrity_endio(struct bio *bio)
> {
> - if (bio_integrity(bio))
> + struct bio_integrity_payload *bip = bio_integrity(bio);
> +
> + if (bip && (bip->bip_flags & BIP_BLOCK_INTEGRITY))
> return __bio_integrity_endio(bio);
Seems we will need similar BIP_BLOCK_INTEGRITY check inside bio_uninit
too. At line 221 in below snippet [1].
As that also frees integrity by calling bio_integrity_free. Earlier that
free was skipped due to BIP_INTEGRITY_USER flag. Now that flag has gone,
integrity will get freed and after that control may go back to
nvme-passthrough where it will try to unmap (and potentially copy back)
integrity.
> return true;
> }
> diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
> index 7b6e687d436de2..a5a8b44e9b118f 100644
> --- a/include/linux/bio-integrity.h
> +++ b/include/linux/bio-integrity.h
> @@ -10,8 +10,7 @@ enum bip_flags {
> BIP_CTRL_NOCHECK = 1 << 2, /* disable HBA integrity checking */
> BIP_DISK_NOCHECK = 1 << 3, /* disable disk integrity checking */
> BIP_IP_CHECKSUM = 1 << 4, /* IP checksum */
> - BIP_INTEGRITY_USER = 1 << 5, /* Integrity payload is user address */
Seems this patch is with "for-6.11/block".
But with "for-next" you will see more places where this flag has been
used. And there will be conflicts because we have this patch there [1].
Parts of this patch need changes to work with it. I can look more and
test next week.
[1]
commit e038ee6189842e9662d2fc59d09dbcf48350cf99
Author: Anuj Gupta <anuj20.g@xxxxxxxxxxx>
Date: Mon Jun 10 16:41:44 2024 +0530
block: unmap and free user mapped integrity via submitter
The user mapped intergity is copied back and unpinned by
bio_integrity_free which is a low-level routine. Do it via the
submitter
rather than doing it in the low-level block layer code, to split the
submitter side from the consumer side of the bio.
[2]
213 void bio_uninit(struct bio *bio)
214 {
215 #ifdef CONFIG_BLK_CGROUP
216 if (bio->bi_blkg) {
217 blkg_put(bio->bi_blkg);
218 bio->bi_blkg = NULL;
219 }
220 #endif
221 if (bio_integrity(bio))
222 bio_integrity_free(bio);
