Messages in this thread | | | Date | Tue, 15 Nov 2022 22:53:14 -0800 | From | Christoph Hellwig <> | Subject | Re: [PATCH v2 -next] iommu/dma: avoid expensive indirect calls for sync operations |
| |
As Robing pointed out this really is mostly a dma-mapping layer patch and the subject should reflect that.
> + if (!IS_ENABLED(CONFIG_DMA_API_DEBUG) && dev_is_dma_coherent(dev)) > + dev->skip_dma_sync = true;
I don't think CONFIG_DMA_API_DEBUG should come into play here. It is independent from the low-level sync calls. That'll need a bit of restructuring later on to only skil the sync calls and not the debug_dma_* calls, but I think that is worth it to keep the concept clean. In fact that might lead to just checking the skip_dma_sync flag in a wrapper in dma-mapping.h, avoiding the function call entirely for the fast path, at the downside of increasing code size by adding an extra branch in the callers, but I think that is ok.
I think we should also apply the skip_dma_sync to dma-direct while we're it. While dma-direct already avoids the indirect calls we can shave off a few more cycles with this infrastructure, so why skip that?
> --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -647,6 +647,7 @@ struct device { > defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) > bool dma_coherent:1; > #endif > + bool skip_dma_sync:1;
This also needs a blurb in the kerneldoc comment describing struct device. Please make it very clear in the comment that the flag is only for internal use in the DMA mapping subsystem and not for use by drives.
> +static inline bool dev_skip_dma_sync(struct device *dev) > +{ > + return dev->skip_dma_sync; > +}
I'd drop this wrapper and just check the flag directly.
> + if (unlikely(dev->skip_dma_sync)) > + dev->skip_dma_sync = false;
Please add a comment here.
Btw, one thing I had in my mind for a while is to do direct calls to dma-iommu from the core mapping code just like we for dma-direct. Would that be useful for your networking loads, or are the actual mapping calls rare enough to not matter?
| |