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