[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240912105009.GI4026@unreal>
Date: Thu, 12 Sep 2024 13:50:09 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
Marek Szyprowski <m.szyprowski@...sung.com>, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org,
NĂcolas F. R. A. Prado <nfraprado@...labora.com>
Subject: Re: [PATCH] dma-mapping: reliably inform about DMA support for IOMMU
On Thu, Sep 12, 2024 at 09:29:54AM +0200, Christoph Hellwig wrote:
> Thanks, I've applied this. I do like the flow of dma_supported even
> less with the duplicate condition now. Can I get a quick review for
> this incremental cleanup, please?
>
> ---
> From d311bfe189d12a836b4add55fdb946f37f8697fa Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@....de>
> Date: Thu, 12 Sep 2024 09:21:18 +0200
> Subject: dma-mapping: reflow dma_supported
>
> dma_supported has become too much spaghetti for my taste. Reflow it to
> remove the duplicate use_dma_iommu condition and make the main path more
> obvious.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
> kernel/dma/mapping.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
Reviewed-by: Leon Romanovsky <leon@...nel.org>
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 022d670f8cad29..cd2a97d362cd24 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -841,20 +841,23 @@ static int dma_supported(struct device *dev, u64 mask)
> {
> const struct dma_map_ops *ops = get_dma_ops(dev);
>
> - if (WARN_ON(ops && use_dma_iommu(dev)))
> - return false;
> -
> - if (use_dma_iommu(dev))
> + if (use_dma_iommu(dev)) {
> + if (WARN_ON(ops))
> + return false;
> return true;
You can write it as "return !WARN_ON(ops);" if you want to be fancy, but
your version is perfectly fine too, so ignore this comment :).
> + }
> +
> /*
> - * ->dma_supported sets the bypass flag, so we must always call
> - * into the method here unless the device is truly direct mapped.
> + * ->dma_supported sets and clears the bypass flag, so ignore it here
> + * and always call into the method if there is one.
> */
> - if (!ops)
> - return dma_direct_supported(dev, mask);
> - if (!ops->dma_supported)
> - return 1;
> - return ops->dma_supported(dev, mask);
> + if (ops) {
> + if (!ops->dma_supported)
> + return 1;
Worth to change it to be "return true;"
> + return ops->dma_supported(dev, mask);
> + }
> +
> + return dma_direct_supported(dev, mask);
> }
>
> bool dma_pci_p2pdma_supported(struct device *dev)
> --
> 2.45.2
>
Powered by blists - more mailing lists