| Message ID | 20240425183943.6319-2-joshi.k@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Read/Write with meta/integrity | expand |
On Fri, Apr 26, 2024 at 12:09:34AM +0530, Kanchan Joshi wrote: > From: Anuj Gupta <anuj20.g@samsung.com> > > Set the bip_vcnt correctly in bio_integrity_init_user and > bio_integrity_copy_user. If the bio gets split at a later point, > this value is required to set the right bip_vcnt in the cloned bio. > > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com> > Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> Please add a Fixes tag and submit it separately from the features. I'm actually kinda surprised the direct user mapping of integrity data survived so far without this.
On Sat, Apr 27, 2024 at 09:02:14AM +0200, Christoph Hellwig wrote: > On Fri, Apr 26, 2024 at 12:09:34AM +0530, Kanchan Joshi wrote: > > From: Anuj Gupta <anuj20.g@samsung.com> > > > > Set the bip_vcnt correctly in bio_integrity_init_user and > > bio_integrity_copy_user. If the bio gets split at a later point, > > this value is required to set the right bip_vcnt in the cloned bio. > > > > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com> > > Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> > > Looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Please add a Fixes tag and submit it separately from the features. > > I'm actually kinda surprised the direct user mapping of integrity data > survived so far without this. The only existing use case for user metadata is REQ_OP_DRV_IN/OUT, which never splits, so these initial fixes only really matter after this series adds new usage for generic READ/WRITE.
On 4/27/2024 7:46 PM, Keith Busch wrote: >> Looks good: >> >> Reviewed-by: Christoph Hellwig<hch@lst.de> >> >> Please add a Fixes tag and submit it separately from the features. >> >> I'm actually kinda surprised the direct user mapping of integrity data >> survived so far without this. > The only existing use case for user metadata is REQ_OP_DRV_IN/OUT, which > never splits, so these initial fixes only really matter after this > series adds new usage for generic READ/WRITE. > Yes. It did not seem that there is any harm due to these missing pieces (first 6 patches). Therefore, "Fixes" tag is not there, and patches were not sent separately from the features.
On Sat, Apr 27, 2024 at 08:16:52AM -0600, Keith Busch wrote: > > Please add a Fixes tag and submit it separately from the features. > > > > I'm actually kinda surprised the direct user mapping of integrity data > > survived so far without this. > > The only existing use case for user metadata is REQ_OP_DRV_IN/OUT, which > never splits, so these initial fixes only really matter after this > series adds new usage for generic READ/WRITE. Well, it matters to keep our contract up, even if we're not hitting it. And apparently another user just came out of the woods in dm land..
On Wed, May 01, 2024 at 09:45:44AM +0200, Christoph Hellwig wrote: > On Sat, Apr 27, 2024 at 08:16:52AM -0600, Keith Busch wrote: > > > Please add a Fixes tag and submit it separately from the features. > > > > > > I'm actually kinda surprised the direct user mapping of integrity data > > > survived so far without this. > > > > The only existing use case for user metadata is REQ_OP_DRV_IN/OUT, which > > never splits, so these initial fixes only really matter after this > > series adds new usage for generic READ/WRITE. > > Well, it matters to keep our contract up, even if we're not hitting it. > > And apparently another user just came out of the woods in dm land.. But the bug report from dm has nothing to do with user mapped metadata. That bug existed before that was added, so yeah, patch 5 from this series (or something like it) should be applied on its own.
diff --git a/block/bio-integrity.c b/block/bio-integrity.c index 2e3e8e04961e..e3390424e6b5 100644 --- a/block/bio-integrity.c +++ b/block/bio-integrity.c @@ -254,6 +254,7 @@ static int bio_integrity_copy_user(struct bio *bio, struct bio_vec *bvec, bip->bip_flags |= BIP_INTEGRITY_USER | BIP_COPY_USER; bip->bip_iter.bi_sector = seed; + bip->bip_vcnt = nr_vecs; return 0; free_bip: bio_integrity_free(bio); @@ -275,6 +276,7 @@ static int bio_integrity_init_user(struct bio *bio, struct bio_vec *bvec, bip->bip_flags |= BIP_INTEGRITY_USER; bip->bip_iter.bi_sector = seed; bip->bip_iter.bi_size = len; + bip->bip_vcnt = nr_vecs; return 0; }