From: Michael Kelley <mhkli...@outlook.com> Sent: Thursday, June 27, 2024 8:05 AM > > From: Petr Tesařík <p...@tesarici.cz> Sent: Thursday, June 27, 2024 12:21 AM > > [...] > > > > @@ -187,10 +169,13 @@ static inline bool is_swiotlb_buffer(struct device > > > *dev, phys_addr_t paddr) > > > * This barrier pairs with smp_mb() in swiotlb_find_slots(). > > > */ > > > smp_rmb(); > > > - return READ_ONCE(dev->dma_uses_io_tlb) && > > > - swiotlb_find_pool(dev, paddr); > > > + if (READ_ONCE(dev->dma_uses_io_tlb)) > > > + return swiotlb_find_pool(dev, paddr); > > > + return NULL; > > > #else > > > - return paddr >= mem->defpool.start && paddr < mem->defpool.end; > > > + if (paddr >= mem->defpool.start && paddr < mem->defpool.end) > > > + return &mem->defpool; > > > > Why are we open-coding swiotlb_find_pool() here? It does not make a > > difference now, but if swiotlb_find_pool() were to change, both places > > would have to be updated. > > > > Does it save a reload from dev->dma_io_tlb_mem? IOW is the compiler > > unable to optimize it away? > > > > What about this (functionally identical) variant: > > > > #ifdef CONFIG_SWIOTLB_DYNAMIC > > smp_rmb(); > > if (!READ_ONCE(dev->dma_uses_io_tlb)) > > return NULL; > > #else > > if (paddr < mem->defpool.start || paddr >= mem->defpool.end); > > return NULL; > > #endif > > > > return swiotlb_find_pool(dev, paddr); > > > > Yeah, I see your point. I'll try this and see what the generated code > looks like. It might take me a couple of days to get to it. >
With and without CONFIG_SWIOTLB_DYNAMIC, there's no meaningful difference in the generated code for x86 or for arm64. I'll incorporate this change into v2. Michael
