Re: [PATCH 2/4] Split the free lists into kernel and user parts

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Mon, Jan 23, 2006 at 09:39:16AM +0000, Mel Gorman wrote:
> On Sun, 22 Jan 2006, Marcelo Tosatti wrote:
> 
> > Hi Mel,
> >
> > On Fri, Jan 20, 2006 at 11:54:55AM +0000, Mel Gorman wrote:
> > >
> > > This patch adds the core of the anti-fragmentation strategy. It works by
> > > grouping related allocation types together. The idea is that large groups of
> > > pages that may be reclaimed are placed near each other. The zone->free_area
> > > list is broken into RCLM_TYPES number of lists.
> > >
> > > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> > > Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
> > > diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.16-rc1-mm1-001_antifrag_flags/include/linux/mmzone.h linux-2.6.16-rc1-mm1-002_fragcore/include/linux/mmzone.h
> > > --- linux-2.6.16-rc1-mm1-001_antifrag_flags/include/linux/mmzone.h	2006-01-19 11:21:59.000000000 +0000
> > > +++ linux-2.6.16-rc1-mm1-002_fragcore/include/linux/mmzone.h	2006-01-19 21:51:05.000000000 +0000
> > > @@ -22,8 +22,16 @@
> > >  #define MAX_ORDER CONFIG_FORCE_MAX_ZONEORDER
> > >  #endif
> > >
> > > +#define RCLM_NORCLM 0
> > > +#define RCLM_EASY   1
> > > +#define RCLM_TYPES  2
> > > +
> > > +#define for_each_rclmtype_order(type, order) \
> > > +	for (order = 0; order < MAX_ORDER; order++) \
> > > +		for (type = 0; type < RCLM_TYPES; type++)
> > > +
> > >  struct free_area {
> > > -	struct list_head	free_list;
> > > +	struct list_head	free_list[RCLM_TYPES];
> > >  	unsigned long		nr_free;
> > >  };
> > >
> > > diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.16-rc1-mm1-001_antifrag_flags/include/linux/page-flags.h linux-2.6.16-rc1-mm1-002_fragcore/include/linux/page-flags.h
> > > --- linux-2.6.16-rc1-mm1-001_antifrag_flags/include/linux/page-flags.h	2006-01-19 11:21:59.000000000 +0000
> > > +++ linux-2.6.16-rc1-mm1-002_fragcore/include/linux/page-flags.h	2006-01-19 21:51:05.000000000 +0000
> > > @@ -76,6 +76,7 @@
> > >  #define PG_reclaim		17	/* To be reclaimed asap */
> > >  #define PG_nosave_free		18	/* Free, should not be written */
> > >  #define PG_uncached		19	/* Page has been mapped as uncached */
> > > +#define PG_easyrclm		20	/* Page is in an easy reclaim block */
> > >
> > >  /*
> > >   * Global page accounting.  One instance per CPU.  Only unsigned longs are
> > > @@ -345,6 +346,12 @@ extern void __mod_page_state_offset(unsi
> > >  #define SetPageUncached(page)	set_bit(PG_uncached, &(page)->flags)
> > >  #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
> > >
> > > +#define PageEasyRclm(page)	test_bit(PG_easyrclm, &(page)->flags)
> > > +#define SetPageEasyRclm(page)	set_bit(PG_easyrclm, &(page)->flags)
> > > +#define ClearPageEasyRclm(page)	clear_bit(PG_easyrclm, &(page)->flags)
> > > +#define __SetPageEasyRclm(page)	__set_bit(PG_easyrclm, &(page)->flags)
> > > +#define __ClearPageEasyRclm(page) __clear_bit(PG_easyrclm, &(page)->flags)
> > > +
> >
> > You can't read/write to page->flags non-atomically, except when you
> > guarantee that the page is not visible to other CPU's (eg at the very
> > end of the page freeing code).
> >
> 
> The helper PageEasyRclm is only used when either the spinlock is held or a
> per-cpu page is being released so it should be safe. The Set and Clear
> helpers are only used with a spinlock held.

Mel,

Other codepaths which touch page->flags do not hold any lock, so you
really must use atomic operations, except when you've guarantee that the
page is being freed and won't be reused.