linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>,
	Sagi Grimberg <sagi@grimberg.me>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-nvme@lists.infradead.org
Subject: Re: [PATCH v4 0/2] block: Enable proper MMIO memory handling for P2P DMA
Date: Thu, 13 Nov 2025 21:50:08 +0200	[thread overview]
Message-ID: <20251113195008.GA111768@unreal> (raw)
In-Reply-To: <4f75497d-11cb-437c-ab90-d65d4d2e0a52@kernel.dk>

On Thu, Nov 13, 2025 at 10:45:53AM -0700, Jens Axboe wrote:
> On 11/13/25 10:12 AM, Jens Axboe wrote:
> > On 11/13/25 9:39 AM, Jens Axboe wrote:
> >>
> >> On Wed, 12 Nov 2025 21:48:03 +0200, Leon Romanovsky wrote:
> >>> Changelog:
> >>> v4:
> >>>  * Changed double "if" to be "else if".
> >>>  * Added missed PCI_P2PDMA_MAP_NONE case.
> >>> v3: https://patch.msgid.link/20251027-block-with-mmio-v3-0-ac3370e1f7b7@nvidia.com
> >>>  * Encoded p2p map type in IOD flags instead of DMA attributes.
> >>>  * Removed REQ_P2PDMA flag from block layer.
> >>>  * Simplified map_phys conversion patch.
> >>> v2: https://lore.kernel.org/all/20251020-block-with-mmio-v2-0-147e9f93d8d4@nvidia.com/
> >>>  * Added Chirstoph's Reviewed-by tag for first patch.
> >>>  * Squashed patches
> >>>  * Stored DMA MMIO attribute in NVMe IOD flags variable instead of block layer.
> >>> v1: https://patch.msgid.link/20251017-block-with-mmio-v1-0-3f486904db5e@nvidia.com
> >>>  * Reordered patches.
> >>>  * Dropped patch which tried to unify unmap flow.
> >>>  * Set MMIO flag separately for data and integrity payloads.
> >>> v0: https://lore.kernel.org/all/cover.1760369219.git.leon@kernel.org/
> >>>
> >>> [...]
> >>
> >> Applied, thanks!
> >>
> >> [1/2] nvme-pci: migrate to dma_map_phys instead of map_page
> >>       commit: f10000db2f7cf29d8c2ade69266bed7b51c772cb
> >> [2/2] block-dma: properly take MMIO path
> >>       commit: 8df2745e8b23fdbe34c5b0a24607f5aaf10ed7eb
> > 
> > And now dropped again - this doesn't boot on neither my big test box
> > with 33 nvme drives, nor even on my local test vm. Two different archs,
> > and very different setups. Which begs the question, how on earth was
> > this tested, if it doesn't boot on anything I have here?!
> 
> I took a look, and what happens here is that iter.p2pdma.map is 0 as it
> never got set to anything. That is the same as PCI_P2PDMA_MAP_UNKNOWN,
> and hence we just end up in a BLK_STS_RESOURCE. First of all, returning
> BLK_STS_RESOURCE for that seems... highly suspicious. That should surely
> be a fatal error. And secondly, this just further backs up that there's
> ZERO testing done on this patchset at all. WTF?
> 
> FWIW, the below makes it boot just fine, as expected, as a default zero
> filled iter then matches the UNKNOWN case.
> 
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e5ca8301bb8b..4cce69226773 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1087,6 +1087,7 @@ static blk_status_t nvme_map_data(struct request *req)
>  	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
>  		iod->flags |= IOD_DATA_MMIO;
>  		break;
> +	case PCI_P2PDMA_MAP_UNKNOWN:
>  	case PCI_P2PDMA_MAP_NONE:
>  		break;
>  	default:
> @@ -1122,6 +1123,7 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req)
>  	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
>  		iod->flags |= IOD_META_MMIO;
>  		break;
> +	case PCI_P2PDMA_MAP_UNKNOWN:
>  	case PCI_P2PDMA_MAP_NONE:
>  		break;
>  	default:

Sorry for troubles.

Can you please squash this fixup instead?
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index 98554929507a..807048644f2e 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -172,6 +172,7 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
 
        memset(&iter->p2pdma, 0, sizeof(iter->p2pdma));
        iter->status = BLK_STS_OK;
+       iter->p2pdma.map = PCI_P2PDMA_MAP_NONE;
 
        /*
         * Grab the first segment ASAP because we'll need it to check for P2P




> 
> -- 
> Jens Axboe

  reply	other threads:[~2025-11-13 19:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-12 19:48 [PATCH v4 0/2] block: Enable proper MMIO memory handling for P2P DMA Leon Romanovsky
2025-11-12 19:48 ` [PATCH v4 1/2] nvme-pci: migrate to dma_map_phys instead of map_page Leon Romanovsky
2025-11-12 23:57   ` Chaitanya Kulkarni
2025-11-12 19:48 ` [PATCH v4 2/2] block-dma: properly take MMIO path Leon Romanovsky
2025-11-12 20:00   ` Keith Busch
2025-11-12 23:57   ` Chaitanya Kulkarni
2025-11-13 16:39 ` [PATCH v4 0/2] block: Enable proper MMIO memory handling for P2P DMA Jens Axboe
2025-11-13 17:12   ` Jens Axboe
2025-11-13 17:45     ` Jens Axboe
2025-11-13 19:50       ` Leon Romanovsky [this message]
2025-11-13 20:03         ` Keith Busch
2025-11-13 20:13           ` Leon Romanovsky
2025-11-13 20:40             ` Leon Romanovsky
2025-11-13 20:43         ` Jens Axboe
2025-11-14  8:13           ` Leon Romanovsky
2025-11-13 19:52       ` Keith Busch
2025-11-13 20:40         ` Jens Axboe
2025-11-14  8:16           ` Leon Romanovsky
2025-11-14 12:08             ` Jens Axboe

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=20251113195008.GA111768@unreal \
    --to=leon@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /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 NNTP newsgroup(s).