[PATCH 0/2] blk-mq-dma: p2p cleanups and integrity fixup

[PATCH 0/2] blk-mq-dma: p2p cleanups and integrity fixup

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

After reviewing and discussing with Leon and Jason on some of the
proposed updates, it became clean that having the callers track p2p dma
usage doesn't scale, and we have a convenient way to track these things
in the existing structures anyway. So, thank goodness nvme is the only
client of this API at the moment while we work out the kinks.

While looking at it, I also noticed that integrity wouldn't attempt to
use p2p pages, which I think was simply an oversight.

Keith Busch (2):
  blk-integrity: enable p2p source and destination
  blk-mq-dma: bring back p2p request flags

 block/bio-integrity.c         | 21 +++++++++++++++++----
 block/blk-mq-dma.c            |  4 ++++
 drivers/nvme/host/pci.c       | 21 ++++-----------------
 include/linux/bio-integrity.h |  1 +
 include/linux/blk-integrity.h | 14 ++++++++++++++
 include/linux/blk-mq-dma.h    | 11 +++++++++--
 include/linux/blk_types.h     |  2 ++
 7 files changed, 51 insertions(+), 23 deletions(-)

-- 
2.47.3

[PATCH 1/2] blk-integrity: enable p2p source and destination

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

User provided metadata can come from anywhere, including p2p pages. Set
the extraction flags to allow p2p pages for the metadata buffer if the
block device allows it. Similar to data payloads, ensure the bio
disallows merging if we see a p2p page.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 block/bio-integrity.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 6b077ca937f6b..5709ce2bf3464 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -230,7 +230,8 @@ static int bio_integrity_init_user(struct bio *bio, struct bio_vec *bvec,
 }
 
 static unsigned int bvec_from_pages(struct bio_vec *bvec, struct page **pages,
-				    int nr_vecs, ssize_t bytes, ssize_t offset)
+				    int nr_vecs, ssize_t bytes, ssize_t offset,
+				    bool *is_p2p)
 {
 	unsigned int nr_bvecs = 0;
 	int i, j;
@@ -251,6 +252,9 @@ static unsigned int bvec_from_pages(struct bio_vec *bvec, struct page **pages,
 			bytes -= next;
 		}
 
+		if (is_pci_p2pdma_page(page))
+			*is_p2p = true;
+
 		bvec_set_page(&bvec[nr_bvecs], pages[i], size, offset);
 		offset = 0;
 		nr_bvecs++;
@@ -265,10 +269,11 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 	unsigned int align = blk_lim_dma_alignment_and_pad(&q->limits);
 	struct page *stack_pages[UIO_FASTIOV], **pages = stack_pages;
 	struct bio_vec stack_vec[UIO_FASTIOV], *bvec = stack_vec;
+	iov_iter_extraction_t extraction_flags = 0;
 	size_t offset, bytes = iter->count;
+	bool copy, is_p2p = false;
 	unsigned int nr_bvecs;
 	int ret, nr_vecs;
-	bool copy;
 
 	if (bio_integrity(bio))
 		return -EINVAL;
@@ -286,15 +291,23 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 	}
 
 	copy = !iov_iter_is_aligned(iter, align, align);
-	ret = iov_iter_extract_pages(iter, &pages, bytes, nr_vecs, 0, &offset);
+
+	if (blk_queue_pci_p2pdma(q))
+		extraction_flags |= ITER_ALLOW_P2PDMA;
+
+	ret = iov_iter_extract_pages(iter, &pages, bytes, nr_vecs,
+					extraction_flags, &offset);
 	if (unlikely(ret < 0))
 		goto free_bvec;
 
-	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset);
+	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
+				   &is_p2p);
 	if (pages != stack_pages)
 		kvfree(pages);
 	if (nr_bvecs > queue_max_integrity_segments(q))
 		copy = true;
+	if (is_p2p)
+		bio->bi_opf |= REQ_NOMERGE;
 
 	if (copy)
 		ret = bio_integrity_copy_user(bio, bvec, nr_bvecs, bytes);
-- 
2.47.3

Re: [PATCH 1/2] blk-integrity: enable p2p source and destination

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

We only need to consider data and metadata dma mapping types separately.
The request and bio integrity payload have enough flag bits to
internally track the mapping type for each. Add flags for these so the
caller doesn't need to track them, and provide separete request and
integrity helpers to the common code for unmpaping. This will make it
easier to scale as new mappings are added without burdening the caller
to track such things.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 block/bio-integrity.c         |  2 +-
 block/blk-mq-dma.c            |  4 ++++
 drivers/nvme/host/pci.c       | 21 ++++-----------------
 include/linux/bio-integrity.h |  1 +
 include/linux/blk-integrity.h | 14 ++++++++++++++
 include/linux/blk-mq-dma.h    | 11 +++++++++--
 include/linux/blk_types.h     |  2 ++
 7 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 5709ce2bf3464..d3618ed106f4e 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -252,7 +252,7 @@ static unsigned int bvec_from_pages(struct bio_vec *bvec, struct page **pages,
 			bytes -= next;
 		}
 
-		if (is_pci_p2pdma_page(page))
+		if (is_pci_p2pdma_page(pages[i]))
 			*is_p2p = true;
 
 		bvec_set_page(&bvec[nr_bvecs], pages[i], size, offset);
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 660b5e200ccf6..449950029872a 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -174,6 +174,10 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
 	switch (pci_p2pdma_state(&iter->p2pdma, dma_dev,
 				 phys_to_page(vec.paddr))) {
 	case PCI_P2PDMA_MAP_BUS_ADDR:
+		if (iter->iter.is_integrity)
+			bio_integrity(req->bio)->bip_flags |= BIP_P2P_DMA;
+		else
+			req->cmd_flags |= REQ_P2PDMA;
 		return blk_dma_map_bus(iter, &vec);
 	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
 		/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index d8a9dee55de33..28e203b894eb1 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -260,12 +260,6 @@ enum nvme_iod_flags {
 	/* single segment dma mapping */
 	IOD_SINGLE_SEGMENT	= 1U << 2,
 
-	/* DMA mapped with PCI_P2PDMA_MAP_BUS_ADDR */
-	IOD_P2P_BUS_ADDR	= 1U << 3,
-
-	/* Metadata DMA mapped with PCI_P2PDMA_MAP_BUS_ADDR */
-	IOD_META_P2P_BUS_ADDR	= 1U << 4,
-
 	/* Metadata using non-coalesced MPTR */
 	IOD_SINGLE_META_SEGMENT	= 1U << 5,
 };
@@ -737,9 +731,8 @@ static void nvme_unmap_metadata(struct request *req)
 		return;
 	}
 
-	if (!blk_rq_dma_unmap(req, dma_dev, &iod->meta_dma_state,
-				iod->meta_total_len,
-				iod->flags & IOD_META_P2P_BUS_ADDR)) {
+	if (!blk_rq_integrity_dma_unmap(req, dma_dev, &iod->meta_dma_state,
+					iod->meta_total_len)) {
 		if (nvme_pci_cmd_use_meta_sgl(&iod->cmd))
 			nvme_free_sgls(req, sge, &sge[1]);
 		else
@@ -766,8 +759,7 @@ static void nvme_unmap_data(struct request *req)
 		return;
 	}
 
-	if (!blk_rq_dma_unmap(req, dma_dev, &iod->dma_state, iod->total_len,
-				iod->flags & IOD_P2P_BUS_ADDR)) {
+	if (!blk_rq_dma_unmap(req, dma_dev, &iod->dma_state, iod->total_len)) {
 		if (nvme_pci_cmd_use_sgl(&iod->cmd))
 			nvme_free_sgls(req, iod->descriptors[0],
 				       &iod->cmd.common.dptr.sgl);
@@ -1043,9 +1035,6 @@ static blk_status_t nvme_map_data(struct request *req)
 	if (!blk_rq_dma_map_iter_start(req, dev->dev, &iod->dma_state, &iter))
 		return iter.status;
 
-	if (iter.p2pdma.map == PCI_P2PDMA_MAP_BUS_ADDR)
-		iod->flags |= IOD_P2P_BUS_ADDR;
-
 	if (use_sgl == SGL_FORCED ||
 	    (use_sgl == SGL_SUPPORTED &&
 	     (sgl_threshold && nvme_pci_avg_seg_size(req) >= sgl_threshold)))
@@ -1068,9 +1057,7 @@ static blk_status_t nvme_pci_setup_meta_sgls(struct request *req)
 						&iod->meta_dma_state, &iter))
 		return iter.status;
 
-	if (iter.p2pdma.map == PCI_P2PDMA_MAP_BUS_ADDR)
-		iod->flags |= IOD_META_P2P_BUS_ADDR;
-	else if (blk_rq_dma_map_coalesce(&iod->meta_dma_state))
+	if (blk_rq_dma_map_coalesce(&iod->meta_dma_state))
 		entries = 1;
 
 	/*
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 0a25716820fe0..851254f36eb36 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -13,6 +13,7 @@ enum bip_flags {
 	BIP_CHECK_GUARD		= 1 << 5, /* guard check */
 	BIP_CHECK_REFTAG	= 1 << 6, /* reftag check */
 	BIP_CHECK_APPTAG	= 1 << 7, /* apptag check */
+	BIP_P2P_DMA		= 1 << 8, /* using P2P address */
 };
 
 struct bio_integrity_payload {
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index 78fe2459e6612..4746f3e6ad36d 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -27,6 +27,15 @@ static inline bool queue_limits_stack_integrity_bdev(struct queue_limits *t,
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 int blk_rq_map_integrity_sg(struct request *, struct scatterlist *);
+
+static inline bool blk_rq_integrity_dma_unmap(struct request *req,
+		struct device *dma_dev, struct dma_iova_state *state,
+		size_t mapped_len)
+{
+	return blk_dma_unmap(req, dma_dev, state, mapped_len,
+			bio_integrity(req->bio)->bip_flags & BIP_P2P_DMA);
+}
+
 int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
 int blk_rq_integrity_map_user(struct request *rq, void __user *ubuf,
 			      ssize_t bytes);
@@ -115,6 +124,11 @@ static inline int blk_rq_map_integrity_sg(struct request *q,
 {
 	return 0;
 }
+static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
+		struct dma_iova_state *state, size_t mapped_len)
+{
+	return false;
+}
 static inline int blk_rq_integrity_map_user(struct request *rq,
 					    void __user *ubuf,
 					    ssize_t bytes)
diff --git a/include/linux/blk-mq-dma.h b/include/linux/blk-mq-dma.h
index 0f45ea110ca12..51829958d8729 100644
--- a/include/linux/blk-mq-dma.h
+++ b/include/linux/blk-mq-dma.h
@@ -43,7 +43,7 @@ static inline bool blk_rq_dma_map_coalesce(struct dma_iova_state *state)
 }
 
 /**
- * blk_rq_dma_unmap - try to DMA unmap a request
+ * blk_dma_unmap - try to DMA unmap a request
  * @req:	request to unmap
  * @dma_dev:	device to unmap from
  * @state:	DMA IOVA state
@@ -53,7 +53,7 @@ static inline bool blk_rq_dma_map_coalesce(struct dma_iova_state *state)
  * Returns %false if the callers need to manually unmap every DMA segment
  * mapped using @iter or %true if no work is left to be done.
  */
-static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
+static inline bool blk_dma_unmap(struct request *req, struct device *dma_dev,
 		struct dma_iova_state *state, size_t mapped_len, bool is_p2p)
 {
 	if (is_p2p)
@@ -68,4 +68,11 @@ static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
 	return !dma_need_unmap(dma_dev);
 }
 
+static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
+		struct dma_iova_state *state, size_t mapped_len)
+{
+	return blk_dma_unmap(req, dma_dev, state, mapped_len,
+				req->cmd_flags & REQ_P2PDMA);
+}
+
 #endif /* BLK_MQ_DMA_H */
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 930daff207df2..09b99d52fd365 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -386,6 +386,7 @@ enum req_flag_bits {
 	__REQ_DRV,		/* for driver use */
 	__REQ_FS_PRIVATE,	/* for file system (submitter) use */
 	__REQ_ATOMIC,		/* for atomic write operations */
+	__REQ_P2PDMA,		/* contains P2P DMA pages */
 	/*
 	 * Command specific flags, keep last:
 	 */
@@ -418,6 +419,7 @@ enum req_flag_bits {
 #define REQ_DRV		(__force blk_opf_t)(1ULL << __REQ_DRV)
 #define REQ_FS_PRIVATE	(__force blk_opf_t)(1ULL << __REQ_FS_PRIVATE)
 #define REQ_ATOMIC	(__force blk_opf_t)(1ULL << __REQ_ATOMIC)
+#define REQ_P2PDMA	(__force blk_opf_t)(1ULL << __REQ_P2PDMA)
 
 #define REQ_NOUNMAP	(__force blk_opf_t)(1ULL << __REQ_NOUNMAP)
 
-- 
2.47.3

Re: [PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Keith Busch]

On Fri, Aug 29, 2025 at 07:23:07AM -0700, Keith Busch wrote:
> @@ -252,7 +252,7 @@ static unsigned int bvec_from_pages(struct bio_vec *bvec, struct page **pages,
>  			bytes -= next;
>  		}
>  
> -		if (is_pci_p2pdma_page(page))
> +		if (is_pci_p2pdma_page(pages[i]))
>  			*is_p2p = true;

Oh no... >_<

This part should have been folded into patch 1.

Re: [PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Christoph Hellwig]

On Fri, Aug 29, 2025 at 07:23:07AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> We only need to consider data and metadata dma mapping types separately.
> The request and bio integrity payload have enough flag bits to
> internally track the mapping type for each. Add flags for these so the
> caller doesn't need to track them, and provide separete request and
> integrity helpers to the common code for unmpaping. This will make it
> easier to scale as new mappings are added without burdening the caller
> to track such things.

We are actually about to run out of REQ_* bits with the current
encoding.  We could shrink the space for REQ_OP_ a bit to create
more, or try to move some flags out into BIO_ flags (like
REQ_ALLOC_CACHE) or kill them by looking at pointers instead
(REQ_INTEGRITY), or by overlaying flags that can't be used with
the same of (REQ_FUA vs REQ_RAHEAD vs REQ_UNMAP for example).
And maybe we can come up with a more coherent scheme for
REQ_PRIO / REQ_BACKGROUND / REQ_SWAP and maybe REQ_IDLE that create
another priority scheme in addition to the I/O priorities.

Re: [PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Keith Busch]

On Tue, Sep 02, 2025 at 07:33:58AM +0200, Christoph Hellwig wrote:
> On Fri, Aug 29, 2025 at 07:23:07AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > We only need to consider data and metadata dma mapping types separately.
> > The request and bio integrity payload have enough flag bits to
> > internally track the mapping type for each. Add flags for these so the
> > caller doesn't need to track them, and provide separete request and
> > integrity helpers to the common code for unmpaping. This will make it
> > easier to scale as new mappings are added without burdening the caller
> > to track such things.
> 
> We are actually about to run out of REQ_* bits with the current
> encoding.  We could shrink the space for REQ_OP_ a bit to create
> more, or try to move some flags out into BIO_ flags (like
> REQ_ALLOC_CACHE) or kill them by looking at pointers instead
> (REQ_INTEGRITY), or by overlaying flags that can't be used with
> the same of (REQ_FUA vs REQ_RAHEAD vs REQ_UNMAP for example).
> And maybe we can come up with a more coherent scheme for
> REQ_PRIO / REQ_BACKGROUND / REQ_SWAP and maybe REQ_IDLE that create
> another priority scheme in addition to the I/O priorities.
 
Sure, but can we do that effort separately from this? I'm mainly trying
to align with Leon's DMA series that adds REQ_MMIO so that we won't have
flag conflicts.

Re: [PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Christoph Hellwig]

On Tue, Sep 02, 2025 at 08:36:51AM -0600, Keith Busch wrote:
> > We are actually about to run out of REQ_* bits with the current
> > encoding.  We could shrink the space for REQ_OP_ a bit to create
> > more, or try to move some flags out into BIO_ flags (like
> > REQ_ALLOC_CACHE) or kill them by looking at pointers instead
> > (REQ_INTEGRITY), or by overlaying flags that can't be used with
> > the same of (REQ_FUA vs REQ_RAHEAD vs REQ_UNMAP for example).
> > And maybe we can come up with a more coherent scheme for
> > REQ_PRIO / REQ_BACKGROUND / REQ_SWAP and maybe REQ_IDLE that create
> > another priority scheme in addition to the I/O priorities.
>  
> Sure, but can we do that effort separately from this? I'm mainly trying
> to align with Leon's DMA series that adds REQ_MMIO so that we won't have
> flag conflicts.

I was just thinking out aloud how we could reclaim them, not trying
to take that on for this series.

Re: [PATCH 2/2] blk-mq-dma: bring back p2p request flags

[Leon Romanovsky]

On Tue, Sep 02, 2025 at 08:36:51AM -0600, Keith Busch wrote:
> On Tue, Sep 02, 2025 at 07:33:58AM +0200, Christoph Hellwig wrote:
> > On Fri, Aug 29, 2025 at 07:23:07AM -0700, Keith Busch wrote:
> > > From: Keith Busch <kbusch@kernel.org>
> > > 
> > > We only need to consider data and metadata dma mapping types separately.
> > > The request and bio integrity payload have enough flag bits to
> > > internally track the mapping type for each. Add flags for these so the
> > > caller doesn't need to track them, and provide separete request and
> > > integrity helpers to the common code for unmpaping. This will make it
> > > easier to scale as new mappings are added without burdening the caller
> > > to track such things.
> > 
> > We are actually about to run out of REQ_* bits with the current
> > encoding.  We could shrink the space for REQ_OP_ a bit to create
> > more, or try to move some flags out into BIO_ flags (like
> > REQ_ALLOC_CACHE) or kill them by looking at pointers instead
> > (REQ_INTEGRITY), or by overlaying flags that can't be used with
> > the same of (REQ_FUA vs REQ_RAHEAD vs REQ_UNMAP for example).
> > And maybe we can come up with a more coherent scheme for
> > REQ_PRIO / REQ_BACKGROUND / REQ_SWAP and maybe REQ_IDLE that create
> > another priority scheme in addition to the I/O priorities.
>  
> Sure, but can we do that effort separately from this? I'm mainly trying
> to align with Leon's DMA series that adds REQ_MMIO so that we won't have
> flag conflicts.

Christoph,

In addition, let's make sure that functionality is correct and working right.

REQ_* cleanup can be perfect followup series for someone who understands
semantics around these bits.

Thanks