lkml.org 
[lkml]   [2010]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] memory hotplug: use unified logic for is_removable and offline_pages
On Mon 06-09-10 14:47:16, KAMEZAWA Hiroyuki wrote:
>
> Now, sysfs interface of memory hotplug shows whether the section is
> removable or not. But it checks only migrateype of pages and doesn't
> check details of cluster of pages.
>
> Next, memory hotplug's set_migratetype_isolate() has the same kind
> of check, too. But the migrate-type is just a "hint" and the pageblock
> can contain several types of pages if fragmentation is very heavy.
>
> To get precise information, we need to check
> - the pageblock only contains free pages or LRU pages.
>
> This patch adds the function __count_unmovable_pages() and makes
> above 2 checks to use the same logic. This will improve user experience
> of memory hotplug because sysfs interface tells accurate information.
>
> Note:
> it may be better to check MIGRATE_UNMOVABLE for making failure case quick.
>
> Changelog: 2010/09/06
> - added comments.
> - removed zone->lock.
> - changed the name of the function to be is_pageblock_removable_async().
> because I removed the zone->lock.

wouldn't be __is_pageblock_removable a better name? _async suffix is
usually used for asynchronous operations and this is just a function
withtout locks.

>
> Reported-by: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> include/linux/memory_hotplug.h | 1
> mm/memory_hotplug.c | 15 -------
> mm/page_alloc.c | 77 ++++++++++++++++++++++++++++++-----------
> 3 files changed, 60 insertions(+), 33 deletions(-)
>
> Index: kametest/mm/page_alloc.c
> ===================================================================
> --- kametest.orig/mm/page_alloc.c
> +++ kametest/mm/page_alloc.c
> @@ -5274,11 +5274,61 @@ void set_pageblock_flags_group(struct pa
> * page allocater never alloc memory from ISOLATE block.
> */
>

Can we add a comment on the locking? Something like:
Caller should hold zone->lock if he needs consistent results.

> +static int __count_immobile_pages(struct zone *zone, struct page *page)
> +{
> + unsigned long pfn, iter, found;
> + /*
> + * For avoiding noise data, lru_add_drain_all() should be called
> + * If ZONE_MOVABLE, the zone never contains immobile pages
> + */
> + if (zone_idx(zone) == ZONE_MOVABLE)
> + return 0;
> +
> + pfn = page_to_pfn(page);
> + for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) {
> + unsigned long check = pfn + iter;
> +
> + if (!pfn_valid_within(check)) {
> + iter++;
> + continue;
> + }
> + page = pfn_to_page(check);
> + if (!page_count(page)) {
> + if (PageBuddy(page))
> + iter += (1 << page_order(page)) - 1;
> + continue;
> + }
> + if (!PageLRU(page))
> + found++;
> + /*
> + * If the page is not RAM, page_count()should be 0.
> + * we don't need more check. This is an _used_ not-movable page.
> + *
> + * The problematic thing here is PG_reserved pages. PG_reserved
> + * is set to both of a memory hole page and a _used_ kernel
> + * page at boot.
> + */
> + }
> + return found;
> +}
> +
> +bool is_pageblock_removable_async(struct page *page)
> +{
> + struct zone *zone = page_zone(page);
> + unsigned long flags;
> + int num;
> + /* Don't take zone->lock interntionally. */

Could you add the reason?
Don't take zone-> lock intentionally because we are called from the
userspace (sysfs interface).

[...]
> /* All pageblocks in the memory block are likely to be hot-removable */
> Index: kametest/include/linux/memory_hotplug.h
> ===================================================================
> --- kametest.orig/include/linux/memory_hotplug.h
> +++ kametest/include/linux/memory_hotplug.h
> @@ -69,6 +69,7 @@ extern void online_page(struct page *pag
> /* VM interface that may be used by firmware interface */
> extern int online_pages(unsigned long, unsigned long);
> extern void __offline_isolated_pages(unsigned long, unsigned long);

#ifdef CONFIG_HOTREMOVE

> +extern bool is_pageblock_removable_async(struct page *page);

#else
#define is_pageblock_removable_async(p) 0
#endif
?

Thanks!
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic


\
 
 \ /
  Last update: 2010-09-06 11:33    [W:0.341 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site