Re: [RFC PATCH] dma-iommu: Add iommu_dma_max_mapping_size()

From: Robin Murphy
Date: Tue May 17 2022 - 08:02:58 EST


On 2022-05-17 12:26, John Garry wrote:
On 17/05/2022 11:40, Robin Murphy wrote:
On 2022-05-16 14:06, John Garry wrote:
For streaming DMA mappings involving an IOMMU and whose IOVA len regularly
exceeds the IOVA rcache upper limit (meaning that they are not cached),
performance can be reduced.

Add the IOMMU callback for DMA mapping API dma_max_mapping_size(), which
allows the drivers to know the mapping limit and thus limit the requested
IOVA lengths.

This resolves the performance issue originally reported in [0] for a SCSI
HBA driver which was regularly mapping SGLs which required IOVAs in
excess of the IOVA caching limit. In this case the block layer limits the
max sectors per request - as configured in __scsi_init_queue() - which
will limit the total SGL length the driver tries to map and in turn limits
IOVA lengths requested.

[0] https://lore.kernel.org/linux-iommu/20210129092120.1482-1-thunder.leizhen@xxxxxxxxxx/

Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
Sending as an RFC as iommu_dma_max_mapping_size() is a soft limit, and not
a hard limit which I expect is the semantics of dma_map_ops.max_mapping_size

Indeed, sorry but NAK for this being nonsense. As I've said at least once before, if the unnecessary SAC address allocation attempt slows down your workload, make it not do that in the first place. If you don't like the existing command-line parameter then fine, > there are plenty of
other options, it just needs to be done in a way that doesn't break x86 systems with dodgy firmware, as my first attempt turned out to.

Sorry, but I am not interested in this. It was discussed in Jan last year without any viable solution.

Er, OK, if you're not interested in solving that problem I don't see why you'd bring it up, but hey ho. *I* still think it's important, so I guess I'll revive my old patch with a CONFIG_X86 bodge and have another go at some point.

Anyway we still have the long-term IOVA aging issue, and requesting non-cached IOVAs is involved in that. So I would rather keep the SCSI driver to requesting cached IOVAs all the time.

I did try to do it the other way around - configuring the IOVA caching range according to the drivers requirement but that got nowhere.

FWIW I thought that all looked OK, it just kept getting drowned out by more critical things in my inbox so I hoped someone else might comment. If it turns out that I've become the de-facto IOVA maintainer in everyone else's minds now and they're all waiting for my word then fair enough, I just need to know and reset my expectations accordingly. Joerg?

Furthermore, if a particular SCSI driver doesn't benefit from mappings larger than 256KB, then that driver is also free to limit its own mapping size. There are other folks out there with use-cases for mapping *gigabytes* at once; you don't get to cripple the API and say that that's suddenly not allowed just because it happens to make your thing go faster, that's absurd.

I'd say less catastrophically slow, not faster.

So how to inform the SCSI driver of this caching limit then so that it may limit the SGL length?

Driver-specific mechanism; block-layer-specific mechanism; redefine this whole API to something like dma_opt_mapping_size(), as a limit above which mappings might become less efficient or start to fail (callback to my thoughts on [1] as well, I suppose); many options. Just not imposing a ridiculously low *maximum* on everyone wherein mapping calls "should not be larger than the returned value" when that's clearly bollocks.

Cheers,
Robin.

[1] https://lore.kernel.org/linux-iommu/20220510142109.777738-1-ltykernel@xxxxxxxxx/