Re: [PATCH 3/7] Allow CONFIG_MIGRATION to be set without CONFIG_NUMA

[Mel Gorman <mel@csn.ul.ie>]

On Tue, 29 May 2007, Christoph Lameter wrote:

> On Tue, 29 May 2007, Mel Gorman wrote:
>
> > CONFIG_MIGRATION currently depends on CONFIG_NUMA. move_pages() is the only
> > user of migration today and as this system call is only meaningful on NUMA,
> > it makes sense. However, memory compaction will operate within a zone and is
> > useful on both NUMA and non-NUMA systems. This patch allows CONFIG_MIGRATION
> > to be used in all memory models. To preserve existing behaviour, move_pages()
> > is only available when CONFIG_NUMA is set.
>
> Hmmm... I thought I had this already set up so that it would be easy to
> switch page migration to not depend on CONFIG_NUMA. Not so it seems.

It's only policy_zone that it got hung-up on. In an earlir version, I just 
defined policy_zone outside of mempolicy but it was messy looking.

> > --- linux-2.6.22-rc2-mm1-005_migrate_nocontext/include/linux/migrate.h	2007-05-28 14:11:32.000000000 +0100
> > +++ linux-2.6.22-rc2-mm1-015_migration_flatmem/include/linux/migrate.h	2007-05-29 10:00:09.000000000 +0100
> > @@ -7,7 +7,7 @@
> >
> >  typedef struct page *new_page_t(struct page *, unsigned long private, int **);
> >
> > -#ifdef CONFIG_MIGRATION
> > +#ifdef CONFIG_SYSCALL_MOVE_PAGES
> >  /* Check if a vma is migratable */
> >  static inline int vma_migratable(struct vm_area_struct *vma)
> >  {
> > @@ -24,7 +24,14 @@ static inline int vma_migratable(struct
> >  			return 0;
> >  	return 1;
> >  }
> > +#else
> > +static inline int vma_migratable(struct vm_area_struct *vma)
> > +{
> > +	return 0;
> > +}
> > +#endif
>
> I guess we get compilation failures because of the reference to
> policy_zone here for the !NUMA case? I think vma migratable is not used at
> all if !NUMA.

It isn't that I could tell.

> > +#ifdef CONFIG_MIGRATION
> >  extern int isolate_lru_page(struct page *p, struct list_head *pagelist);
> >  extern int putback_lru_pages(struct list_head *l);
> >  extern int migrate_page(struct address_space *,
> > @@ -40,8 +47,6 @@ extern int migrate_vmas(struct mm_struct
> >  		const nodemask_t *from, const nodemask_t *to,
> >  		unsigned long flags);
> >  #else
> > -static inline int vma_migratable(struct vm_area_struct *vma)
> > -					{ return 0; }
>
> Maybe this block is not necessary?

Agreed. For a long time I didn't have it included at all but put it back 
in to preserve existing behaviour. I'll remove it.

> > diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.22-rc2-mm1-005_migrate_nocontext/include/linux/mm.h linux-2.6.22-rc2-mm1-015_migration_flatmem/include/linux/mm.h
> > --- linux-2.6.22-rc2-mm1-005_migrate_nocontext/include/linux/mm.h	2007-05-24 10:13:34.000000000 +0100
> > +++ linux-2.6.22-rc2-mm1-015_migration_flatmem/include/linux/mm.h	2007-05-28 14:13:44.000000000 +0100
> > @@ -242,6 +242,8 @@ struct vm_operations_struct {
> >  	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
> >  	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,
> >  					unsigned long addr);
> > +#endif /* CONFIG_NUMA */
> > +#ifdef CONFIG_MIGRATION
> >  	int (*migrate)(struct vm_area_struct *vma, const nodemask_t *from,
> >  		const nodemask_t *to, unsigned long flags);
> >  #endif
>
> Correct.
>
> > diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.22-rc2-mm1-005_migrate_nocontext/mm/Kconfig linux-2.6.22-rc2-mm1-015_migration_flatmem/mm/Kconfig
> > --- linux-2.6.22-rc2-mm1-005_migrate_nocontext/mm/Kconfig	2007-05-24 10:13:34.000000000 +0100
> > +++ linux-2.6.22-rc2-mm1-015_migration_flatmem/mm/Kconfig	2007-05-29 09:57:23.000000000 +0100
> > @@ -145,13 +145,16 @@ config SPLIT_PTLOCK_CPUS
> >  config MIGRATION
> >  	bool "Page migration"
> >  	def_bool y
> > -	depends on NUMA
> >  	help
> >  	  Allows the migration of the physical location of pages of processes
> >  	  while the virtual addresses are not changed. This is useful for
> >  	  example on NUMA systems to put pages nearer to the processors accessing
> >  	  the page.
> >
> > +config SYSCALL_MOVE_PAGES
> > +	def_bool y
> > +	depends on MIGRATION && NUMA
> > +
>
> Do we really need the CONFIG_SYSCALL_MOVE_PAGES? I think you will directly
> access the lower levels. So why have it? CONFIG_SYSCALL_MOVE_PAGES ==
> CONFIG_NUMA.

Without SYSCALL_MOVE_PAGES, the check in migrate.h becomes

#if defined(CONFIG_NUMA) && defined(CONFIG_MIGRATION)
/* Check if a vma is migratable */
static inline int vma_migratable(struct vm_area_struct *vma)
#endif

That in itself is fine but in mm/migrate.c I didn't want to define 
sys_move_pages() in the non-NUMA case. Whatever about the header file 
where SYSCALL_MOVE_PAGES obscures understanding, I think it makes sense to 
have SYSCALL_MOVE_PAGES for mm/migrate.c . What do you think?

--
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/