Re: [PATCH] NVMe: Fix compilation on architecturs without readq/writeq

[Hitoshi Mitake]

On Sat, Jan 21, 2012 at 17:28, Ingo Molnar <mingo@xxxxxxx> wrote:
>
> * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> On Thu, Jan 19, 2012 at 5:01 PM, Matthew Wilcox
>> <matthew.r.wilcox@xxxxxxxxx> wrote:
>> > The only places that uses readq/writeq are in the initialisation
>> > path.  Since they're not performance critical, always use readl/writel.
>>
>> The arch rules are that i fthe architecture has readq/writeq, they
>> will be #define'd (they may be inline functions, but there will be a
>>
>>   #define readq readq
>>
>> to make it visible to the preprocessor as well).
>>
>> So if you don't need the atomicity guarantees of a "real" readq, you
>> can do this instead:
>>
>>   #ifndef readq
>>   static inline u64 readq(void __iomem *addr)
>>   {
>>         return readl(addr) | (((u64) readl(addr + 4)) << 32LL);
>>   }
>>   #endif
>>
>> and then use readq() as if it existed.
>>
>> And I do think we should expose this in some generic manner. Because
>> we currently don't, we already have that pattern copied in quite a few
>> drivers.
>>
>> Maybe <asm-generic/io-nonatomic.h> or something? Making it
>> clear that its not atomic, but avoiding the silly duplication
>> we do now..
>>
>> This whole mess was introduced in commit dbee8a0affd5 ("x86:
>> remove 32-bit versions of readq()/writeq()"), and it already
>> talked about the problems but didn't help with the drivers
>> that simply don't care.
>>
>> All the people in those threads were doing their
>> self-satisfied "writeq is broken", without much acknowledging
>> that 99% of users simply don't seem to care.
>>
>> "Occupy Writeq - We are the 99%"
>
> Agreed, and offering a generic facility for silly duplication
> was the motivation of the original commit by Hitoshi Mitake.
>
> This:
>
> | The presense of a writeq() implementation on 32-bit x86 that
> | splits the 64-bit write into two 32-bit writes turns out to
> | break the mpt2sas driver (and in general is risky for drivers
> | as was discussed in
> |   <http://lkml.kernel.org/r/adaab6c1h7c.fsf@xxxxxxxxx>).
>
> is actually a mostly bogus statement and creates more problems
> than it solves.
>
> Hitoshi-san, would you be interested in re-adding the generic
> readq/writeq definitions in a slight variation to 2c5643b1c5, to
> a separate io-nonatomic.h file, so that drivers that want it can
> #include that file and be happy?

It sounds nice. In the previous discussion, I suggested that
chaning the name of non-atomic readq/writeq to
readq_nonatomic/writeq_nonatomic. And James Bottomley
replied that it is fine but not really very useful:

https://lkml.org/lkml/2011/5/19/13

The idea of providing non-atomic readq/writeq in the new file
with the name which express non-atomicity clearly might be
able to satisfy both of safety and usefulness.

It will reduce the duplication of the definition. In addition
readq/writeq users don't have to type the long symbols with
_nonatomic suffix and can know non-atomicity from the name
of header file.

I'd like to hear opinions from James, Roland and folks who
dislike non-atomic readq/writeq.

-- 
Hitoshi Mitake
h.mitake@xxxxxxxxx
--
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/