From: Mikulas Patocka <mpatocka@redhat.com>
To: Jens Axboe <axboe@kernel.dk>, Keith Busch <kbusch@kernel.org>,
Mike Snitzer <snitzer@kernel.org>
Cc: linux-block@vger.kernel.org, dm-devel@lists.linux.dev,
linux-nvme@lists.infradead.org
Subject: [PATCH] block: change rq_integrity_vec to respect the iterator
Date: Mon, 29 Apr 2024 20:37:26 +0200 (CEST) [thread overview]
Message-ID: <19d1b52a-f43e-5b41-ff1d-5257c7b3492@redhat.com> (raw)
Hi
I am changing dm-crypt, so that it can store the autenticated encryption
tag directly into the NVMe metadata (without using dm-integrity). This
will improve performance significantly, because we can avoid journaling
done by dm-integrity. I've got it working, but I've found this bug, so I'm
sending a patch for it.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
If we allocate a bio that is larger than NVMe maximum request size, attach
integrity metadata to it and send it to the NVMe subsystem, the integrity
metadata will be corrupted.
Splitting the bio works correctly. The function bio_split will clone the
bio, trim the size of the first bio and advance the iterator of the second
bio.
However, the function rq_integrity_vec has a bug - it returns the first
vector of the bio's metadata and completely disregards the metadata
iterator that was advanced when the bio was split. Thus, the second bio
uses the same metadata as the first bio and this leads to metadata
corruption.
This commit changes rq_integrity_vec, so that it calls mp_bvec_iter_bvec
instead of returning the first vector. mp_bvec_iter_bvec reads the
iterator and advances the vector by the iterator.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/nvme/host/pci.c | 6 +++---
include/linux/blk-integrity.h | 12 ++++++------
2 files changed, 9 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/nvme/host/pci.c
===================================================================
--- linux-2.6.orig/drivers/nvme/host/pci.c
+++ linux-2.6/drivers/nvme/host/pci.c
@@ -825,9 +825,9 @@ static blk_status_t nvme_map_metadata(st
struct nvme_command *cmnd)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
+ struct bio_vec bv = rq_integrity_vec(req);
- iod->meta_dma = dma_map_bvec(dev->dev, rq_integrity_vec(req),
- rq_dma_dir(req), 0);
+ iod->meta_dma = dma_map_bvec(dev->dev, &bv, rq_dma_dir(req), 0);
if (dma_mapping_error(dev->dev, iod->meta_dma))
return BLK_STS_IOERR;
cmnd->rw.metadata = cpu_to_le64(iod->meta_dma);
@@ -966,7 +966,7 @@ static __always_inline void nvme_pci_unm
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
dma_unmap_page(dev->dev, iod->meta_dma,
- rq_integrity_vec(req)->bv_len, rq_dma_dir(req));
+ rq_integrity_vec(req).bv_len, rq_dma_dir(req));
}
if (blk_rq_nr_phys_segments(req))
Index: linux-2.6/include/linux/blk-integrity.h
===================================================================
--- linux-2.6.orig/include/linux/blk-integrity.h
+++ linux-2.6/include/linux/blk-integrity.h
@@ -109,11 +109,11 @@ static inline bool blk_integrity_rq(stru
* Return the first bvec that contains integrity data. Only drivers that are
* limited to a single integrity segment should use this helper.
*/
-static inline struct bio_vec *rq_integrity_vec(struct request *rq)
+static inline struct bio_vec rq_integrity_vec(struct request *rq)
{
- if (WARN_ON_ONCE(queue_max_integrity_segments(rq->q) > 1))
- return NULL;
- return rq->bio->bi_integrity->bip_vec;
+ WARN_ON_ONCE(queue_max_integrity_segments(rq->q) > 1);
+ return mp_bvec_iter_bvec(rq->bio->bi_integrity->bip_vec,
+ rq->bio->bi_integrity->bip_iter);
}
#else /* CONFIG_BLK_DEV_INTEGRITY */
static inline int blk_rq_count_integrity_sg(struct request_queue *q,
@@ -177,9 +177,9 @@ static inline int blk_integrity_rq(struc
return 0;
}
-static inline struct bio_vec *rq_integrity_vec(struct request *rq)
+static inline struct bio_vec rq_integrity_vec(struct request *rq)
{
- return NULL;
+ BUG();
}
#endif /* CONFIG_BLK_DEV_INTEGRITY */
#endif /* _LINUX_BLK_INTEGRITY_H */
next reply other threads:[~2024-04-29 18:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20240429183739epcas5p1181a2946641784305b188f320033c637@epcas5p1.samsung.com>
2024-04-29 18:37 ` Mikulas Patocka [this message]
2024-04-30 4:49 ` [PATCH] block: change rq_integrity_vec to respect the iterator Anuj gupta
2024-04-30 8:16 ` Keith Busch
2024-05-01 7:49 ` Christoph Hellwig
2024-05-02 11:25 ` Kanchan Joshi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=19d1b52a-f43e-5b41-ff1d-5257c7b3492@redhat.com \
--to=mpatocka@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@lists.linux.dev \
--cc=kbusch@kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=snitzer@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).