On Mon, 14 Jan 2013 21:04:06 +0100, Daniel Vetter <daniel at ffwll.ch> wrote: > On Mon, Jan 14, 2013 at 8:21 PM, Imre Deak <imre.deak at intel.com> wrote: > >> + drm_clflush_virt_range(vaddr + page_offset, 4); > >> + *(uint32_t *)(vaddr + page_offset) = reloc->delta; > >> + drm_clflush_virt_range(vaddr + page_offset, 4); > > > > Discussed this already to some degree with Chris, but I still think the > > first cache flush is redundant. > > Nope, since it's a partial cacheline write, we need to first flush out > any stale data, then write the dword (which loads the latest data from > memory into a cacheline). Then we need to flush the updated cacheline > out into main memory again. Iirc I've mentioned this somewhere in the > part 4 of my gem intro. The question was whether there could be any stale data in that aliased cacheline, and whether or not the code was a continuation of a cargo cult. My answer was that it was our working knowledge that that flush is crucial to unconfuse the CPU. In the absence of a definitive reference for the mysteries of clflush, we should write one. -Chris -- Chris Wilson, Intel Open Source Technology Centre
