Re: [PATCH] asm-generic/io.h: Implement read[bwlq]_relaxed()

[Daniel Thompson]

On 09/09/14 13:28, Will Deacon wrote:
> Hi Daniel,
> 
> On Tue, Sep 09, 2014 at 01:12:40PM +0100, Daniel Thompson wrote:
>> Currently the read[bwlq]_relaxed() family are implemented on every
>> architecture except blackfin, m68k[1], metag, openrisc, s390[2] and
>> score. Increasingly drivers are being optimized to exploit relaxed
>> reads putting these architectures at risk of compilation failures for
>> shared drivers.
>>
>> This patch addresses this by providing implementations of
>> read[bwlq]_relaxed() that are identical to the equivalent read[bwlq]().
>> All the above architectures include asm-generic/io.h .
>>
>> Note that currently only eight architectures (alpha, arm, arm64, avr32,
>> hexagon, microblaze, mips and sh) implement write[bwlq]_relaxed() meaning
>> these functions are deliberately not included in this patch.
>>
>> [1] m68k includes the relaxed family only when configured *without* MMU.
>> [2] s390 requires CONFIG_PCI to include the relaxed family.
>>
>> Signed-off-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
>> Cc: Will Deacon <will.deacon@xxxxxxx>
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Cc: linux-arch@xxxxxxxxxxxxxxx
>> ---
>>  include/asm-generic/io.h | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
> 
> 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:...


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


> Ben and I agreed on something back in May:
> 
>   https://lkml.org/lkml/2014/5/22/468

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

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.


Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/