lkml.org 
[lkml]   [2010]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] mm: page allocator: Update free page counters after pages are placed on the free list
On Fri, Sep 03, 2010 at 03:38:59PM -0700, Andrew Morton wrote:
> On Fri, 3 Sep 2010 10:08:44 +0100
> Mel Gorman <mel@csn.ul.ie> wrote:
>
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -588,12 +588,12 @@ static void free_pcppages_bulk(struct zone *zone, int count,
> > {
> > int migratetype = 0;
> > int batch_free = 0;
> > + int freed = count;
> >
> > spin_lock(&zone->lock);
> > zone->all_unreclaimable = 0;
> > zone->pages_scanned = 0;
> >
> > - __mod_zone_page_state(zone, NR_FREE_PAGES, count);
> > while (count) {
> > struct page *page;
> > struct list_head *list;
> > @@ -621,6 +621,7 @@ static void free_pcppages_bulk(struct zone *zone, int count,
> > trace_mm_page_pcpu_drain(page, 0, page_private(page));
> > } while (--count && --batch_free && !list_empty(list));
> > }
> > + __mod_zone_page_state(zone, NR_FREE_PAGES, freed);
> > spin_unlock(&zone->lock);
> > }
> >
>
> nit: this is why it's evil to modify the value of incoming args. It's
> nicer to leave them alone and treat them as const across the lifetime
> of the callee.
>

Ok, I can see the logic of that.

> Can I do this?
>
> --- a/mm/page_alloc.c~mm-page-allocator-update-free-page-counters-after-pages-are-placed-on-the-free-list-fix
> +++ a/mm/page_alloc.c
> @@ -588,13 +588,13 @@ static void free_pcppages_bulk(struct zo
> {
> int migratetype = 0;
> int batch_free = 0;
> - int freed = count;
> + int to_free = count;
>
> spin_lock(&zone->lock);
> zone->all_unreclaimable = 0;
> zone->pages_scanned = 0;
>
> - while (count) {
> + while (to_free) {
> struct page *page;
> struct list_head *list;
>
> @@ -619,9 +619,9 @@ static void free_pcppages_bulk(struct zo
> /* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
> __free_one_page(page, zone, 0, page_private(page));
> trace_mm_page_pcpu_drain(page, 0, page_private(page));
> - } while (--count && --batch_free && !list_empty(list));
> + } while (--to_free && --batch_free && !list_empty(list));
> }
> - __mod_zone_page_state(zone, NR_FREE_PAGES, freed);
> + __mod_zone_page_state(zone, NR_FREE_PAGES, count);
> spin_unlock(&zone->lock);
> }

Yes you can. I see no problem with this alteration.

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab


\
 
 \ /
  Last update: 2010-09-05 20:09    [W:0.152 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site