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. Michael
