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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 11 Feb 2009 15:18:01 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org, npiggin@...e.de,
	hugh@...itas.com, riel@...hat.com, kosaki.motohiro@...fujitsu.com
Subject: Re: [PATCH 2/2] mm: update_page_reclaim_stat() is called form page
 fault path

On Wed, 11 Feb 2009 21:35:07 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@...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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/