On 10/13/2015 06:35 PM, Dan Williams wrote: > On Mon, Oct 12, 2015 at 10:18 PM, Thomas Hellstrom > <thellstrom at vmware.com> wrote: >> Hi! >> >> On 10/13/2015 12:35 AM, Dan Williams wrote: >>> Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver >>> expects the fifo registers to be cacheable. In preparation for >>> deprecating ioremap_cache() convert its usage in vmwgfx to memremap(). >>> >>> Cc: David Airlie <airlied at linux.ie> >>> Cc: Thomas Hellstrom <thellstrom at vmware.com> >>> Cc: Sinclair Yeh <syeh at vmware.com> >>> Cc: dri-devel at lists.freedesktop.org >>> Signed-off-by: Dan Williams <dan.j.williams at intel.com> >> While I have nothing against the conversion, what's stopping the >> compiler from reordering writes on a generic architecture and caching >> and reordering reads on x86 in particular? At the very least it looks to >> me like the memory accesses of the memremap'd memory needs to be >> encapsulated within READ_ONCE and WRITE_ONCE. > Hmm, currently the code is using ioread32/iowrite32 which only do > volatile accesses, whereas READ_ONCE / WRITE_ONCE have a memory > clobber on entry and exit. So, I'm assuming all you need is the > guarantee of "no compiler re-ordering" and not the stronger > READ_ONCE/WRITE_ONCE guarantees, but that still seems broken compared > to explicit fencing where it matters.
I'm not quite sure I follow you here, it looks to me like READ_ONCE() and WRITE_ONCE() are implemented as volatile accesses, http://lxr.free-electrons.com/source/include/linux/compiler.h#L215 just like ioread32 and iowrite32 http://lxr.free-electrons.com/source/include/asm-generic/io.h#L54 which would minimize any potential impact of this change. IMO optimizing the memory accesses can be done as a later step. /Thomas
