Re: [PATCH v3 2/2] block-dma: properly take MMIO path

Keith Busch
2025-11-12T15:23:26Z

On Wed, Nov 12, 2025 at 05:16:04PM +0200, Leon Romanovsky wrote:
> On Wed, Nov 12, 2025 at 09:38:07AM -0500, Keith Busch wrote:
> > On Mon, Oct 27, 2025 at 09:30:21AM +0200, Leon Romanovsky wrote:
> > > @@ -732,13 +746,20 @@ static void nvme_unmap_metadata(struct request *req)
> > > return;
> > > }
> > >
> > > - if (!blk_rq_integrity_dma_unmap(req, dma_dev, &iod->meta_dma_state,
> > > - iod->meta_total_len)) {
> > > + if (iod->flags & IOD_META_P2P)
> > > + map = PCI_P2PDMA_MAP_BUS_ADDR;
> > > + if (iod->flags & IOD_META_MMIO) {
> > > + map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
> > > + attrs |= DMA_ATTR_MMIO;
> > > + }
> >
> > This should be an 'else if' no?
>
> We can't have both IOD_META_P2P and IOD_META_MMIO. It can be only one or
> IOD_META_P2P or IOD_META_MMIO. In this case "else if' is not necessary.

I get that and I'm not saying this is a bug, but it just looks odd.
You're checking the conditions as if they're independent of each other
when they're not. If the first 'if' is true, there's no need to test the
second one as we know it's false, but this code flow will test it
anyway.