Re: [RFC 1/1] swiotlb: Reduce calls to swiotlb_find_pool()

[Petr Tesařík <petr@xxxxxxxxxxx>]

On Mon, 1 Jul 2024 06:36:15 +0200
"hch@xxxxxx" <hch@xxxxxx> wrote:

> On Sun, Jun 30, 2024 at 02:02:52PM +0000, Michael Kelley wrote:
> > 1) Rename is_swiotlb_buffer() to swiotlb_find_pool(), since it
> > now returns a pool.  A NULL return value indicates that the
> > paddr is not an swiotlb buffer.
> > 
> > 2) Similarly, rename is_xen_swiotlb_buffer() to
> > xen_swiotlb_find_pool()
> > 
> > 3) The existing swiotlb_find_pool() has the same function signature,
> > but it is used only where the paddr is known to be an swiotlb buffer
> > and hence always succeeds. Rename it to __swiotlb_find_pool() as
> > the "internal" version of swiotlb_find_pool().  
> 
> Sounds good.

Agreed. Most importantly, the "nice" name swiotlb_find_pool() is used
for external users. The difference between swiotlb_find_pool() and
__swiotlb_find_pool() is that the former can be used with any device,
and the latter (internal) only with devices that make some use of
swiotlb. The main reason to keep them separate is that the internal
function should not be inlined if CONFIG_SWIOTLB_DYNAMIC=y.

I hope somebody finds my explanation useful when they touch the code
again in a few years from now. ;-)

> > 4) Do you still want is_swiotlb_buffer() as a trivial wrapper around
> > the new swiotlb_find_pool(), for use solely in dma_direct_need_sync()
> > where only a Boolean is needed and not the pool?  
> 
> If there is really just a single caller left we can skip the wrapper,
> otherwise it might be handy.

AFAICS dma_direct_need_sync() is the only such place.

Petr T