Re: [PATCH v2 1/1] swiotlb: Reduce swiotlb pool lookups

[Christoph Hellwig]

Hi Michael,

I like the idea behind this, but can you respin it to avoid some of
the added code duplication.  We have a lot of this pattern:

	pool = swiotlb_find_pool(dev, paddr);
	if (pool)
		swiotlb_foo(dev, ...

duplicated in all three swiotlb users.  If we rename the original
swiotlb_foo to __swiotlb_foo and add a little inline wrapper this is
de-duplicated and also avoids exposing swiotlb_find_pool to the
callers.

If we then stub out swiotlb_find_pool to return NULL for !CONFIG_SWIOTLB,
we also don't need extra stubs for all the __swiotlb_ helpers as the
compiler will eliminate the calls as dead code.

I might be missing something, but what is the reason for using the
lower-level __swiotlb_find_pool in swiotlb_map and xen_swiotlb_map_page?
I can't see a reason why the simple checks in swiotlb_find_pool itself
are either wrong or a performance problem there.  Because if we don't
need these separate calls we can do away with __swiotlb_find_pool
for !CONFIG_SWIOTLB_DYNAMIC and simplify swiotlb_find_pool quite
a bit like this:

	...

	if (!mem)
		return NULL;

	if (IS_ENABLED(CONFIG_SWIOTLB_DYNAMIC)) {
		smp_rmb(); 
		if (!READ_ONCE(dev->dma_uses_io_tlb))
			return NULL;
		return __swiotlb_find_pool(dev, paddr);
	}

	if (paddr < mem->defpool.start || paddr >= mem->defpool.end)
		return NULL;
	return &dev->dma_io_tlb_mem->defpool;


While you're at it please fix the > 80 character lines as this patch
is adding plenty.