[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090610011939.GA5603@localhost>
Date: Wed, 10 Jun 2009 09:19:39 +0800
From: Wu Fengguang <fengguang.wu@...el.com>
To: Mel Gorman <mel@....ul.ie>
Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Rik van Riel <riel@...hat.com>,
Christoph Lameter <cl@...ux-foundation.org>,
"Zhang, Yanmin" <yanmin.zhang@...el.com>,
"linuxram@...ibm.com" <linuxram@...ibm.com>,
linux-mm <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] Properly account for the number of page cache
pages zone_reclaim() can reclaim
On Wed, Jun 10, 2009 at 01:01:41AM +0800, Mel Gorman wrote:
> On NUMA machines, the administrator can configure zone_reclaim_mode that
> is a more targetted form of direct reclaim. On machines with large NUMA
> distances for example, a zone_reclaim_mode defaults to 1 meaning that clean
> unmapped pages will be reclaimed if the zone watermarks are not being met.
>
> There is a heuristic that determines if the scan is worthwhile but the
> problem is that the heuristic is not being properly applied and is basically
> assuming zone_reclaim_mode is 1 if it is enabled.
>
> Historically, once enabled it was depending on NR_FILE_PAGES which may
> include swapcache pages that the reclaim_mode cannot deal with. Patch
> vmscan-change-the-number-of-the-unmapped-files-in-zone-reclaim.patch by
> Kosaki Motohiro noted that zone_page_state(zone, NR_FILE_PAGES) included
> pages that were not file-backed such as swapcache and made a calculation
> based on the inactive, active and mapped files. This is far superior
> when zone_reclaim==1 but if RECLAIM_SWAP is set, then NR_FILE_PAGES is a
> reasonable starting figure.
>
> This patch alters how zone_reclaim() works out how many pages it might be
> able to reclaim given the current reclaim_mode. If RECLAIM_SWAP is set
> in the reclaim_mode it will either consider NR_FILE_PAGES as potential
> candidates or else use NR_{IN}ACTIVE}_PAGES-NR_FILE_MAPPED to discount
> swapcache and other non-file-backed pages. If RECLAIM_WRITE is not set,
> then NR_FILE_DIRTY number of pages are not candidates. If RECLAIM_SWAP is
> not set, then NR_FILE_MAPPED are not.
>
> Signed-off-by: Mel Gorman <mel@....ul.ie>
> Acked-by: Christoph Lameter <cl@...ux-foundation.org>
> ---
> mm/vmscan.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 2ddcfc8..2bfc76e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2333,6 +2333,41 @@ int sysctl_min_unmapped_ratio = 1;
> */
> int sysctl_min_slab_ratio = 5;
>
> +static inline unsigned long zone_unmapped_file_pages(struct zone *zone)
> +{
> + return zone_page_state(zone, NR_INACTIVE_FILE) +
> + zone_page_state(zone, NR_ACTIVE_FILE) -
> + zone_page_state(zone, NR_FILE_MAPPED);
This may underflow if too many tmpfs pages are mapped.
> +}
> +
> +/* Work out how many page cache pages we can reclaim in this reclaim_mode */
> +static inline long zone_pagecache_reclaimable(struct zone *zone)
> +{
> + long nr_pagecache_reclaimable;
> + long delta = 0;
> +
> + /*
> + * If RECLAIM_SWAP is set, then all file pages are considered
> + * potentially reclaimable. Otherwise, we have to worry about
> + * pages like swapcache and zone_unmapped_file_pages() provides
> + * a better estimate
> + */
> + if (zone_reclaim_mode & RECLAIM_SWAP)
> + nr_pagecache_reclaimable = zone_page_state(zone, NR_FILE_PAGES);
> + else
> + nr_pagecache_reclaimable = zone_unmapped_file_pages(zone);
> +
> + /* If we can't clean pages, remove dirty pages from consideration */
> + if (!(zone_reclaim_mode & RECLAIM_WRITE))
> + delta += zone_page_state(zone, NR_FILE_DIRTY);
> +
> + /* Beware of double accounting */
The double accounting happens for NR_FILE_MAPPED but not
NR_FILE_DIRTY(dirty tmpfs pages won't be accounted), so this comment
is more suitable for zone_unmapped_file_pages(). But the double
accounting does affects this abstraction. So a more reasonable
sequence could be to first substract NR_FILE_DIRTY and then
conditionally substract NR_FILE_MAPPED?
Or better to introduce a new counter NR_TMPFS_MAPPED to fix this mess?
Thanks,
Fengguang
> + if (delta < nr_pagecache_reclaimable)
> + nr_pagecache_reclaimable -= delta;
> +
> + return nr_pagecache_reclaimable;
> +}
> +
> /*
> * Try to free up some pages from this zone through reclaim.
> */
> @@ -2355,7 +2390,6 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
> .isolate_pages = isolate_pages_global,
> };
> unsigned long slab_reclaimable;
> - long nr_unmapped_file_pages;
>
> disable_swap_token();
> cond_resched();
> @@ -2368,11 +2402,7 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
> reclaim_state.reclaimed_slab = 0;
> p->reclaim_state = &reclaim_state;
>
> - nr_unmapped_file_pages = zone_page_state(zone, NR_INACTIVE_FILE) +
> - zone_page_state(zone, NR_ACTIVE_FILE) -
> - zone_page_state(zone, NR_FILE_MAPPED);
> -
> - if (nr_unmapped_file_pages > zone->min_unmapped_pages) {
> + if (zone_pagecache_reclaimable(zone) > zone->min_unmapped_pages) {
> /*
> * Free memory by calling shrink zone with increasing
> * priorities until we have enough memory freed.
> @@ -2419,8 +2449,6 @@ int zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
> {
> int node_id;
> int ret;
> - long nr_unmapped_file_pages;
> - long nr_slab_reclaimable;
>
> /*
> * Zone reclaim reclaims unmapped file backed pages and
> @@ -2432,12 +2460,8 @@ int zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
> * if less than a specified percentage of the zone is used by
> * unmapped file backed pages.
> */
> - nr_unmapped_file_pages = zone_page_state(zone, NR_INACTIVE_FILE) +
> - zone_page_state(zone, NR_ACTIVE_FILE) -
> - zone_page_state(zone, NR_FILE_MAPPED);
> - nr_slab_reclaimable = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
> - if (nr_unmapped_file_pages <= zone->min_unmapped_pages &&
> - nr_slab_reclaimable <= zone->min_slab_pages)
> + if (zone_pagecache_reclaimable(zone) <= zone->min_unmapped_pages &&
> + zone_page_state(zone, NR_SLAB_RECLAIMABLE) <= zone->min_slab_pages)
> return 0;
>
> if (zone_is_all_unreclaimable(zone))
> --
> 1.5.6.5
--
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/
Powered by blists - more mailing lists