On Wed, Oct 23, 2019 at 7:00 AM Bhanusree <bhanusreemahesh@xxxxxxxxx> wrote:
>
> -Add comment for memory barrier
> -Issue found using checkpatch.pl
>
> Signed-off-by: Bhanusree <bhanusreemahesh@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_cache.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
> index 3bd76e9..39910f2 100644
> --- a/drivers/gpu/drm/drm_cache.c
> +++ b/drivers/gpu/drm/drm_cache.c
> @@ -62,10 +62,10 @@ static void drm_cache_flush_clflush(struct page *pages[],
> {
> unsigned long i;
>
> - mb();
> + mb(); /*make sure page address is read*/
That's not what's going on here. We need the barriers because:
- clflush is an unordered instruction (very rare on x86, but they exist)
- hence we need t have a barrier before&after to make sure the clflush
actually happens when we want it to happen.
- clflush flushes the cpu cache, _that_ is what we want to synchronize
against. not the memory address itself. Hence we need to make sure
that before all reads/writes have hit the cpu cache, and afterwards
that all the cache lines we wanted to flush are flushed.
- https://c9x.me/x86/html/file_module_x86_id_30.html for more info on clflush
So useful comment (see also the link) would be:
/* CLFLUSH is only ordered with a full memory mbarrier (MFENCE
instruction) before ...*/
> for (i = 0; i < num_pages; i++)
> drm_clflush_page(*pages++);
> - mb();
> + mb(); /*make sure all addresses of pages are read sequentially*/
/* ... and after the CLFLUSH instruction itself */
If you dig through all the macros for mb() eventually you should find
the inline assembler for MFENCE. That's also why we need to have this
barrier even on single-processor builds (so no smp_mb()).
Can you pls respin?
Thanks, Daniel
> }
> #endif
>
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
