On Tue, Sep 09, 2014 at 02:14:54PM +0100, Daniel Thompson wrote:
On 09/09/14 14:03, Daniel Thompson wrote:
On 09/09/14 13:28, Will Deacon wrote:
I have a larger series adding these (and the write equivalents) to all architectures that I periodically post and then fail to get on top of.
That's why you're on Cc:...
Ok, so why not just pick the asm-generic patch out of my series?
The key part you're missing is defining some generic semantics for these accessors. Without those, I don't think it makes sense to put them into asm-generic, because drivers can't safely infer any meaning from the relaxed definition.
Currently the semantics are described as: --- cut here --- PCI ordering rules also guarantee that PIO read responses arrive after any outstanding DMA writes from that bus, since for some devices the result of a readb call may signal to the driver that a DMA transaction is complete. In many cases, however, the driver may want to indicate that the next readb call has no relation to any previous DMA writes performed by the device. The driver can use readb_relaxed for these cases, although only some platforms will honor the relaxed semantics. Using the relaxed read functions will provide significant performance benefits on platforms that support it. The qla2xxx driver provides examples of how to use readX_relaxed . In many cases, a majority of the driver’s readX calls can safely be converted to readX_relaxed calls, since only a few will indicate or depend on DMA completion. --- cut here ---
The implementation provided in the patch trivially meets this definition (by not honouring the relaxedness).
I still think we need to mention ordering of relaxed reads against each other and also against spinlocks.
Ben and I agreed on something back in May:
... and didn't you also conclude with hpa that the very relaxed x86 implementation of readl_relaxed() already meets this definition (as do these changes to asm-generic/io.h).
Sorry. "very relaxed" is always a very stupid thing to say about x86 (especially to an arm guy).
More exactly I was referring to the absence of memory clobber in x86 readl_relaxed().
Yeah, my series just adds the relaxed write accessors for x86.
Thus allowing its use to perculate more widely really shouldn't do an harm.
but I need to send a new version including:
- ioreadX_relaxed and iowriteX_relaxed
- Strengthening non-relaxed I/O accessors on architectures with non-empty mmiowb()
I'll bump it up the list. In the meantime, you can have a look at my io branch on kernel.org
I'd really like to see your work included (which I spotted after I wrote the patch and when it occured to me to visit https://www.google.com/search?q=asm-generic+readl_relaxed to see if there was a well known reason not to make this change).
However... I really can't see why we should delay introducing an already documented function to the remaining architectures.
I'd just rather fix the interface once instead of churning it about. How about I dust off the series again?
Will