Re: [PATCH net-next 1/7] dma: compile-out DMA sync op calls when not used

From: Robin Murphy
Date: Mon Jan 29 2024 - 07:15:33 EST


On 2024-01-29 11:07 am, Alexander Lobakin wrote:
From: Christoph Hellwig <hch@xxxxxx>
Date: Mon, 29 Jan 2024 07:11:36 +0100

On Fri, Jan 26, 2024 at 02:54:50PM +0100, Alexander Lobakin wrote:
Some platforms do have DMA, but DMA there is always direct and coherent.
Currently, even on such platforms DMA sync operations are compiled and
called.
Add a new hidden Kconfig symbol, DMA_NEED_SYNC, and set it only when
either sync operations are needed or there is DMA ops or swiotlb
enabled. Set dma_need_sync() and dma_skip_sync() (stub for now)
depending on this symbol state and don't call sync ops when
dma_skip_sync() is true.
The change allows for future optimizations of DMA sync calls depending
on compile-time or runtime conditions.

So the idea of compiling out the calls sounds fine to me. But what
is the point of the extra indirection through the __-prefixed calls?

Because dma_sync_* ops are external functions, not inlines, and in the
next patch I'm adding a check there.


And if we need that (please document it in the commit log), please
make the wrappers proper inline functions and not macros.

In fact those wrappers could perhaps subsume the existing stub definitions, by starting with a refactor along these lines:

static inline dma_sync_x(...)
{
if (IS_ENABLED(CONFIG_NEED_DMA_SYNC))
__dma_sync_x(...);
}

Cheers,
Robin.