Re: [PATCH/RFC 5/14] Reclaim Scalability: Use an indexed array for LRU variables

[Balbir Singh <balbir@linux.vnet.ibm.com>]

Lee Schermerhorn wrote:
> [PATCH/RFC] 05/15  Reclaim Scalability:   Use an indexed array for LRU variables
> 
> From clameter@sgi.com Wed Aug 29 11:39:51 2007
> 
> Currently we are defining explicit variables for the inactive
> and active list. An indexed array can be more generic and avoid
> repeating similar code in several places in the reclaim code.
> 
> We are saving a few bytes in terms of code size:
> 
> Before:
> 
>    text    data     bss     dec     hex filename
> 4097753  573120 4092484 8763357  85b7dd vmlinux
> 
> After:
> 
>    text    data     bss     dec     hex filename
> 4097729  573120 4092484 8763333  85b7c5 vmlinux
> 
> Having an easy way to add new lru lists may ease future work on
> the reclaim code.
> 
> [CL's signoff added by lts based on mail from CL]
> Signed-off-by:  Christoph Lameter <clameter@sgi.com>
> 
>  include/linux/mm_inline.h |   33 ++++++++---
>  include/linux/mmzone.h    |   17 +++--
>  mm/page_alloc.c           |    9 +--
>  mm/swap.c                 |    2 
>  mm/vmscan.c               |  132 ++++++++++++++++++++++------------------------
>  mm/vmstat.c               |    3 -
>  6 files changed, 107 insertions(+), 89 deletions(-)
> 
> Index: Linux/include/linux/mmzone.h
> ===================================================================
> --- Linux.orig/include/linux/mmzone.h	2007-09-10 12:21:31.000000000 -0400
> +++ Linux/include/linux/mmzone.h	2007-09-10 12:22:33.000000000 -0400
> @@ -81,8 +81,8 @@ struct zone_padding {
>  enum zone_stat_item {
>  	/* First 128 byte cacheline (assuming 64 bit words) */
>  	NR_FREE_PAGES,
> -	NR_INACTIVE,
> -	NR_ACTIVE,
> +	NR_INACTIVE,	/* must match order of LRU_[IN]ACTIVE */
> +	NR_ACTIVE,	/*  "     "     "   "       "         */
>  	NR_ANON_PAGES,	/* Mapped anonymous pages */
>  	NR_FILE_MAPPED,	/* pagecache pages mapped into pagetables.
>  			   only modified from process context */
> @@ -106,6 +106,13 @@ enum zone_stat_item {
>  #endif
>  	NR_VM_ZONE_STAT_ITEMS };
> 
> +enum lru_list {
> +	LRU_INACTIVE,	/* must match order of NR_[IN]ACTIVE */
> +	LRU_ACTIVE,	/*  "     "     "   "       "        */
> +	NR_LRU_LISTS };
> +
> +#define for_each_lru(l) for (l = 0; l < NR_LRU_LISTS; l++)
> +
>  struct per_cpu_pages {
>  	int count;		/* number of pages in the list */
>  	int high;		/* high watermark, emptying needed */
> @@ -259,10 +266,8 @@ struct zone {
> 
>  	/* Fields commonly accessed by the page reclaim scanner */
>  	spinlock_t		lru_lock;	
> -	struct list_head	active_list;
> -	struct list_head	inactive_list;
> -	unsigned long		nr_scan_active;
> -	unsigned long		nr_scan_inactive;
> +	struct list_head	list[NR_LRU_LISTS];
> +	unsigned long		nr_scan[NR_LRU_LISTS];

I wonder if it makes sense to have an array of the form

struct reclaim_lists {
	struct list_head list[NR_LRU_LISTS];
	unsigned long nr_scan[NR_LRU_LISTS];
	reclaim_function_t list_reclaim_function[NR_LRU_LISTS];
}

where reclaim_function is an array of reclaim functions for each list
(in our case shrink_active_list/shrink_inactive_list).


>  static inline void
>  del_page_from_lru(struct zone *zone, struct page *page)
>  {
> +	enum lru_list l = LRU_INACTIVE;
> +
>  	list_del(&page->lru);
>  	if (PageActive(page)) {
>  		__ClearPageActive(page);
>  		__dec_zone_state(zone, NR_ACTIVE);
> -	} else {
> -		__dec_zone_state(zone, NR_INACTIVE);
> +		l = LRU_ACTIVE;
>  	}
> +	__dec_zone_state(zone, NR_INACTIVE + l);

This is unconditional, does not seem right.

>  }
> 



-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

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