Re: [PATCH 16/16] mm: filemap: Prefetch page->flags if !PageUptodate

[Hugh Dickins <hughd@google.com>]

On Fri, 18 Apr 2014, Mel Gorman wrote:

> The write_end handler is likely to call SetPageUptodate which is an atomic
> operation so prefetch the line.
> 
> Signed-off-by: Mel Gorman <mgorman@suse.de>

This one seems a little odd to me: it feels as if you're compensating
for your mark_page_accessed() movement, but in too shmem-specific a way.

I see write_ends do SetPageUptodate more often than I was expecting
(with __block_commit_write() doing so even when PageUptodate already),
but even so...

Given that the write_end is likely to want to SetPageDirty, and sure
to want to clear_bit_unlock(PG_locked, &page->flags), wouldn't it be
better and less mysterious just to prefetchw(&page->flags) here
unconditionally?

(But I'm also afraid that this sets a precedent for an avalanche of
dubious prefetchw patches all over.)

Hugh

> ---
>  mm/filemap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index c28f69c..40713da 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2551,6 +2551,9 @@ again:
>  		copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes);
>  		flush_dcache_page(page);
>  
> +		if (!PageUptodate(page))
> +			prefetchw(&page->flags);
> +
>  		status = a_ops->write_end(file, mapping, pos, bytes, copied,
>  						page, fsdata);
>  		if (unlikely(status < 0))
> -- 
> 1.8.4.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>