Re: [PATCH 2/2] mm: update_page_reclaim_stat() is called form page fault path

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 11 Feb 2009 21:35:07 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> 
> Unfortunately, following two patch have a bit conflicted concept.
>   1. commit 9ff473b9a72942c5ac0ad35607cae28d8d59ed7a 
>      (vmscan: evict streaming IO first)
>   2. commit bf3f3bc5e734706730c12a323f9b2068052aa1f0
>      (mm: don't mark_page_accessed in fault path)
> 
> (1) require page fault update reclaim stat via mark_page_accessed(), but
> (2) removed mark_page_accessed() perfectly. 
> 
> However, (1) actually only need to update reclaim stat, but not activate page.
> Then, fault-path calling update_page_reclaim_stat() solve thsi confliction.
> 
> ...
>
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1545,6 +1545,7 @@ retry_find:
>  	/*
>  	 * Found the page and have a reference on it.
>  	 */
> +	update_page_reclaim_stat(page);
>  	ra->prev_pos = (loff_t)page->index << PAGE_CACHE_SHIFT;
>  	vmf->page = page;
>  	return ret | VM_FAULT_LOCKED;

This is the minor fault hotpath.

> +void update_page_reclaim_stat(struct page *page)
> +{
> +	struct zone *zone = page_zone(page);
> +
> +	spin_lock_irq(&zone->lru_lock);
> +	/* if the page isn't reclaimable, it doesn't update reclaim stat */
> +	if (PageLRU(page) && !PageUnevictable(page)) {
> +		update_page_reclaim_stat_locked(zone, page,
> +					 !!page_is_file_cache(page), 1);
> +	}
> +	spin_unlock_irq(&zone->lru_lock);
> +}

And we just added a spin_lock_irq() and a bunch of other stuff to it.

Can we improve this?

Can we just omit it, even?

Can we update those stats locklessly and accomodate the resulting
inaccuracy over at the codesites where these statistics are actually
used?


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