linux-kernel - Re: [RFC] memory hotremove patch take 2 [04/10] (isolate all free pages)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070511095818.240c6f47.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Fri, 11 May 2007 09:58:18 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Mel Gorman <mel@....ul.ie>
Cc:	y-goto@...fujitsu.com, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, akpm@...l.org, clameter@....com
Subject: Re: [RFC] memory hotremove patch take 2 [04/10] (isolate all free
 pages)

On Thu, 10 May 2007 17:42:54 +0100 (IST)
Mel Gorman <mel@....ul.ie> wrote:

> > +	if (!pfn_valid(pfn))
> > +		return -EINVAL;
> 
> This may lead to boundary cases where pages cannot be captured at the 
> start and end of non-aligned zones due to memory holes.
> 
Hm, ok. maybe we can remove this.

> > +	zone = info->zone;
> > +	if ((zone != page_zone(pfn_to_page(pfn))) ||
> > +	    (zone != page_zone(pfn_to_page(last_pfn))))
> > +		return -EINVAL;
> 
> Is this check really necessary? Surely a caller to 
> capture_isolate_freed_pages() will have already made all the necessary 
> checks when adding the struct insolation_info ?
> 
just because isolation_info is treated per zone.
Maybe MIGRATE_ISOLATING can allow us more flexible approach.


> > +	drain_all_pages();
> > +	spin_lock(&zone->lock);
> > +	while (pfn < info->end_pfn) {
> > +		if (!pfn_valid(pfn)) {
> > +			pfn++;
> > +			continue;
> > +		}
> > +		page = pfn_to_page(pfn);
> > +		/* See page_is_buddy()  */
> > +		if (page_count(page) == 0 && PageBuddy(page)) {
> 
> If PageBuddy is set it's free, you shouldn't have to check the page_count.
> 
ok.

> > +			order = page_order(page);
> > +			order_size = 1 << order;
> > +			zone->free_area[order].nr_free--;
> > +			__mod_zone_page_state(zone, NR_FREE_PAGES, -order_size);
> > +			list_del(&page->lru);
> > +			rmv_page_order(page);
> > +			isolate_page_nolock(info, page, order);
> > +			nr_pages += order_size;
> > +			pfn += order_size;
> > +		} else {
> > +			pfn++;
> > +		}
> > +	}
> > +	spin_unlock(&zone->lock);
> > +	return nr_pages;
> > +}
> > #endif /* CONFIG_PAGE_ISOLATION */
> >
> 
> This is all similar to move_freepages() other than the locking part. It 
> would be worth checking if there is code that could be shared or at least 
> have similar styles.

Thank you, I'll look into move_freepages().

-Kame


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/