Messages in this thread | | | Date | Thu, 14 Sep 2023 19:28:01 +0100 | From | Catalin Marinas <> | Subject | Re: [PATCH] swiotlb: fix the check whether a device has used software IO TLB |
| |
On Wed, Sep 13, 2023 at 02:26:56PM +0200, Petr Tesařík wrote: > On Wed, 13 Sep 2023 14:14:03 +0200 > Christoph Hellwig <hch@lst.de> wrote: > > Thanks, I've applied this to get it into linux-next ASAP. I'd love > > to have reviews before sending it on to Linus, though. > > Fully understood, given my past record. ;-) > > @Catalin Marinas: I have added you to the list of recipient, because you > spotted some issues with memory barriers in one of my previous attempts.
I seem to have forgotten everything about that thread (trying to remember what I meant in https://lore.kernel.org/all/ZGJdtmP13pv06xDH@arm.com/ ;)).
What do the smp_wmb() barriers in swiotlb_find_slots() and swiotlb_dyn_alloc() order? The latter is even more unclear as it's at the end of the function and the "pairing" comment doesn't help.
I found the "pairing" description in memory-barriers.txt not helping much and lots of code comments end up stating that some barrier is paired with another as if it is some magic synchronisation event. In general a wmb() barrier ends up paired with an rmb() one (or some other form of dependency, acquire semantics etc.) but they should be described in isolation first - what memory accesses on a CPU before and after the *mb() need ordering - and then how this pairing solves the accesses observability across multiple CPUs. The comments should also describe things like "ensure payload is visible before flag update" rather than "the wmb here pairs with the rmb over there".
Presumably in swiotlb_find_slots(), the smp_wmb() needs to ensure the write to dev->dma_uses_io_tlb is visible before the *retpool write? For swiotlb_dyn_alloc(), I'm completely lost on what the smp_wmb() orders. You really need at least two memory accesses on a CPU for the barrier to make sense.
-- Catalin
| |