Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-11-01 Thread Mel Gorman
On Tue, Nov 01, 2011 at 07:06:56PM +0100, Michal Nazarewicz wrote: > >page_isolation.c may also be a better fit than page_alloc.c > > Since isolate_freepages_block() is the only user of split_free_page(), > would it make sense to move split_free_page() to page_isolation.c as > well? I sort of lik

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-11-01 Thread Michal Nazarewicz
On Tue, 01 Nov 2011 16:04:48 +0100, Mel Gorman wrote: For the purposes of review, have a separate patch for moving isolate_freepages_block to another file that does not alter the function in any way. When the function is updated in a follow-on patch, it'll be far easier to see what has changed.

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-11-01 Thread Mel Gorman
On Sun, Oct 23, 2011 at 09:05:05PM -0700, Michal Nazarewicz wrote: > > On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote: > >> This commit introduces alloc_contig_freed_pages() function > >> which allocates (ie. removes from buddy system) free pages > >> in range. Caller has to guara

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-23 Thread Michal Nazarewicz
> On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote: >> This commit introduces alloc_contig_freed_pages() function >> which allocates (ie. removes from buddy system) free pages >> in range. Caller has to guarantee that all pages in range >> are in buddy system. On Tue, 18 Oct 2011 0

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-23 Thread Michal Nazarewicz
On Fri, 21 Oct 2011 03:06:24 -0700, Mel Gorman wrote: On Tue, Oct 18, 2011 at 10:26:37AM -0700, Michal Nazarewicz wrote: On Tue, 18 Oct 2011 05:21:09 -0700, Mel Gorman wrote: >At this point, I'm going to apologise for not reviewing this a long long >time ago. > >On Thu, Oct 06, 2011 at 03:54

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-21 Thread Mel Gorman
On Tue, Oct 18, 2011 at 10:26:37AM -0700, Michal Nazarewicz wrote: > On Tue, 18 Oct 2011 05:21:09 -0700, Mel Gorman wrote: > > >At this point, I'm going to apologise for not reviewing this a long long > >time ago. > > > >On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote: > >>From:

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-18 Thread Michal Nazarewicz
On Tue, 18 Oct 2011 10:48:46 -0700, Dave Hansen wrote: On Tue, 2011-10-18 at 10:26 -0700, Michal Nazarewicz wrote: > You can do this in a more general fashion by checking the > zone boundaries and resolving the pfn->page every MAX_ORDER_NR_PAGES. > That will not be SPARSEMEM specific. I've tr

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-18 Thread Dave Hansen
On Tue, 2011-10-18 at 10:26 -0700, Michal Nazarewicz wrote: > > You can do this in a more general fashion by checking the > > zone boundaries and resolving the pfn->page every MAX_ORDER_NR_PAGES. > > That will not be SPARSEMEM specific. > > I've tried doing stuff that way but it ended up with much

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-18 Thread Michal Nazarewicz
On Tue, 18 Oct 2011 05:21:09 -0700, Mel Gorman wrote: At this point, I'm going to apologise for not reviewing this a long long time ago. On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote: From: KAMEZAWA Hiroyuki This commit introduces alloc_contig_freed_pages() function which

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-18 Thread Mel Gorman
At this point, I'm going to apologise for not reviewing this a long long time ago. On Thu, Oct 06, 2011 at 03:54:42PM +0200, Marek Szyprowski wrote: > From: KAMEZAWA Hiroyuki > > This commit introduces alloc_contig_freed_pages() function > which allocates (ie. removes from buddy system) free pag

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-17 Thread Andrew Morton
On Mon, 17 Oct 2011 14:21:07 +0200 Marek Szyprowski wrote: > > > + > > > +void free_contig_pages(unsigned long pfn, unsigned nr_pages) > > > +{ > > > + struct page *page = pfn_to_page(pfn); > > > + > > > + while (nr_pages--) { > > > + __free_page(page); > > > + ++pfn; > > > +

RE: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-17 Thread Marek Szyprowski
Hello Andrew, Thanks for your comments. I will try to address them in the next round of CMA patches. On Saturday, October 15, 2011 1:30 AM Andrew Morton wrote: (snipped) > > + > > +void free_contig_pages(unsigned long pfn, unsigned nr_pages) > > +{ > > + struct page *page = pfn_to_page(pfn);

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-16 Thread Michal Nazarewicz
On Sun, 16 Oct 2011 10:01:36 +0200 "Michal Nazarewicz" wrote: Still, as I think of it now, maybe alloc_contig_free_range() would be better? On Sun, 16 Oct 2011 10:31:16 +0200, Andrew Morton wrote: Nope. Of *course* the pages were free. Otherwise we couldn't (re)allocate them. I still think

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-16 Thread Andrew Morton
On Sun, 16 Oct 2011 10:01:36 +0200 "Michal Nazarewicz" wrote: > Still, as I think of it now, maybe alloc_contig_free_range() would be > better? Nope. Of *course* the pages were free. Otherwise we couldn't (re)allocate them. I still think the "free" part is redundant. What could be improved

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-16 Thread Michal Nazarewicz
On Sat, 15 Oct 2011 01:29:33 +0200, Andrew Morton wrote: On Thu, 06 Oct 2011 15:54:42 +0200 Marek Szyprowski wrote: From: KAMEZAWA Hiroyuki This commit introduces alloc_contig_freed_pages() function The "freed" seems redundant to me. Wouldn't "alloc_contig_pages" be a better name? Th

Re: [PATCH 2/9] mm: alloc_contig_freed_pages() added

2011-10-14 Thread Andrew Morton
On Thu, 06 Oct 2011 15:54:42 +0200 Marek Szyprowski wrote: > From: KAMEZAWA Hiroyuki > > This commit introduces alloc_contig_freed_pages() function The "freed" seems redundant to me. Wouldn't "alloc_contig_pages" be a better name? > which allocates (ie. removes from buddy system) free pages