On Tue, Mar 27, 2018 at 12:09 PM, Will Deacon <will.deacon@xxxxxxx> wrote:
> On Tue, Mar 27, 2018 at 12:05:06PM +0200, Arnd Bergmann wrote:
>> > -
>> > - See Documentation/DMA-API.txt for more information on consistent memory.
>> > + can see it now has ownership. Note that, when using writel(), a prior
>> > + wmb() is not needed to guarantee that the cache coherent memory writes
>> > + have completed before writing to the cache incoherent MMIO region.
>> > + If this ordering between incoherent MMIO and coherent memory regions
One more thing: I think the term "incoherent MMIO" is a bit confusing, I'd
prefer just "MMIO" here. At least I don't have the faintest clue what the
difference between "coherent MMIO" and "incoherent MMIO" would be ;-)
>> > + is not required, writel_relaxed() can be used instead and is significantly
>> > + cheaper on some weakly-ordered architectures.
>>
>> I think that's a great improvement, but I'm a bit worried about recommending
>> writel_relaxed() too much: I've seen a lot of drivers that just always use
>> writel_relaxed() over write(), and some of them get that wrong when they
>> don't understand the difference but end up using DMA without explicit
>> barriers anyway.
>>
>> Also, having an architecture-independent driver use wmb()+writel_relaxed()
>> ends up being more expensive than just using write(). Not sure how to
>> best phrase it though.
>
> Perhaps I add reword that with a simple example to say:
>
> If this ordering between incoherent MMIO and coherent memory regions
> is not required (e.g. in a sequence of accesses all to the MMIO region)
> [...]
>
> since that seems to be the usual case where the _relaxed accessors help.
That still doesn't quite capture what I'd like driver writes to do: in essence
I would recommend them to use writel() all the time, except in performance
critical code that has been shown to be correct and has a comment to explain
why _relaxed() is ok in that particular function.
Maybe it can just be rephrased to warn against the use of writel_relaxed()
here, and explain the difference that way:
can see it now has ownership. Note that, when using writel(), a prior
wmb() is not needed to guarantee that the cache coherent memory writes
have completed before writing to the cache incoherent MMIO region.
The cheaper writel_relaxed() does not guarantee the DMA to be visible
to the device and must not be used here.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
