On 26/01/2024 4:45 pm, Alexander Lobakin wrote:
From: Robin Murphy <robin.murphy@xxxxxxx>
Date: Fri, 26 Jan 2024 15:48:54 +0000
On 26/01/2024 1:54 pm, Alexander Lobakin wrote:
From: Eric Dumazet <edumazet@xxxxxxxxxx>
Quite often, NIC devices do not need dma_sync operations on x86_64
at least.
Indeed, when dev_is_dma_coherent(dev) is true and
dev_use_swiotlb(dev) is false, iommu_dma_sync_single_for_cpu()
and friends do nothing.
However, indirectly calling them when CONFIG_RETPOLINE=y consumes about
10% of cycles on a cpu receiving packets from softirq at ~100Gbit rate.
Even if/when CONFIG_RETPOLINE is not set, there is a cost of about 3%.
Add dev->skip_dma_sync boolean which is set during the device
initialization depending on the setup: dev_is_dma_coherent() for direct
DMA, !(sync_single_for_device || sync_single_for_cpu) or positive result
from the new callback, dma_map_ops::can_skip_sync for non-NULL DMA ops.
Then later, if/when swiotlb is used for the first time, the flag
is turned off, from swiotlb_tbl_map_single().
I think you could probably just promote the dma_uses_io_tlb flag from
SWIOTLB_DYNAMIC to a general SWIOTLB thing to serve this purpose now.
Nice catch!
Similarly I don't think a new op is necessary now that we have
dma_map_ops.flags. A simple static flag to indicate that sync may be> skipped under the same conditions as implied for dma-direct - i.e.
dev_is_dma_coherent(dev) && !dev->dma_use_io_tlb - seems like it ought
to suffice.
In my initial implementation, I used a new dma_map_ops flag, but then I
realized different DMA ops may require or not require syncing under
different conditions, not only dev_is_dma_coherent().
Or am I wrong and they would always be the same?
I think it's safe to assume that, as with P2P support, this will only
matter for dma-direct and iommu-dma for the foreseeable future, and
those do currently share the same conditions as above. Thus we may as
well keep things simple for now, and if anything ever does have cause to
change, it can be the future's problem to keep this mechanism working as
intended.
Thanks,
Robin.
