RE: [PATCH] NVMe: Fix compilation on architecturs withoutreadq/writeq

[Wilcox, Matthew R]

Yep, I knew I could check for readq with an ifdef, but I'd rather just use the 32-bit versions everywhere and have consistent behaviour between bittedness.  If this were a performance path, I'd absolutely do what you suggest.

One minor point is that '|' is not a sequence point, so it's not defined whether you'll read the top or bottom half of the register first.  And some hardware people are crazy enough to care.

For this particular hardware, it's defined to work if you read the low order bits first, so I went with the nvme_readq() function to emphasise that this solution works for the nvme driver.

-----Original Message-----
From: linus971@xxxxxxxxx [mailto:linus971@xxxxxxxxx] On Behalf Of Linus Torvalds
Sent: Thursday, January 19, 2012 5:22 PM
To: Wilcox, Matthew R; Roland Dreier; Andrew Morton; Hitoshi Mitake; James Bottomley; Ingo Molnar
Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-nvme@xxxxxxxxxxxxx; hpa@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] NVMe: Fix compilation on architecturs without readq/writeq

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%"

                Linus
--
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/