Re: [PATCH 2/4] cma: count free CMA pages

[Michal Nazarewicz <mina86@mina86.com>]

[-- Attachment #1: Type: text/plain, Size: 4992 bytes --]

Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> writes:
> Add NR_FREE_CMA_PAGES counter to be later used for checking watermark
> in __zone_watermark_ok().  For simplicity and to avoid #ifdef hell make
> this counter always available (not only when CONFIG_CMA=y).

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e9bbd7c..e28e506 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -559,6 +559,9 @@ static inline void __free_one_page(struct page *page,
>  			clear_page_guard_flag(buddy);
>  			set_page_private(page, 0);
>  			__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
> +			if (is_cma_pageblock(page))

Is reading pageblock's type necessary here?  You have migratetype
variable.

> +				__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +						      1 << order);
>  		} else {
>  			list_del(&buddy->lru);
>  			zone->free_area[order].nr_free--;
> @@ -674,6 +677,8 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>  			/* 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));
> +			if (is_cma_pageblock(page))
> +				__mod_zone_page_state(zone, NR_FREE_CMA_PAGES, 1);

Like above, I think that checking page_private(page) should be enough.

>  		} while (--to_free && --batch_free && !list_empty(list));
>  	}
>  	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
> @@ -688,8 +693,12 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
>  	zone->pages_scanned = 0;
>  
>  	__free_one_page(page, zone, order, migratetype);
> -	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE)
> +	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE) {
>  		__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
> +		if (is_cma_pageblock(page))

You are reading pageblock's migratetype twice.  Please use temporary
variable.

> +			__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +					      1 << order);
> +	}
>  	spin_unlock(&zone->lock);
>  }
>  
> @@ -756,6 +765,11 @@ void __meminit __free_pages_bootmem(struct page *page, unsigned int order)
>  }
>  
>  #ifdef CONFIG_CMA
> +bool is_cma_pageblock(struct page *page)
> +{
> +	return get_pageblock_migratetype(page) == MIGRATE_CMA;
> +}
> +
>  /* Free whole pageblock and set it's migration type to MIGRATE_CMA. */
>  void __init init_cma_reserved_pageblock(struct page *page)
>  {
> @@ -813,6 +827,9 @@ static inline void expand(struct zone *zone, struct page *page,
>  			set_page_private(&page[size], high);
>  			/* Guard pages are not available for any usage */
>  			__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << high));
> +			if (is_cma_pageblock(&page[size]))

Like before, why not is_migrate_cma(migratetype)?

> +				__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +						      -(1 << high));
>  			continue;
>  		}
>  #endif
> @@ -1414,8 +1434,12 @@ int split_free_page(struct page *page, bool check_wmark)
>  	zone->free_area[order].nr_free--;
>  	rmv_page_order(page);
>  
> -	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE)
> +	if (get_pageblock_migratetype(page) != MIGRATE_ISOLATE) {
>  		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
> +		if (is_cma_pageblock(page))
> +			__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +					      -(1UL << order));
> +	}

Please use temporary variable. :)

>  
>  	/* Split into individual pages */
>  	set_page_refcounted(page);

> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index d210cc8..b8dba12 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -77,11 +77,15 @@ int set_migratetype_isolate(struct page *page)
>  out:
>  	if (!ret) {
>  		unsigned long nr_pages;
> +		int mt = get_pageblock_migratetype(page);
>  
>  		set_pageblock_isolate(page);
>  		nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
>  
>  		__mod_zone_page_state(zone, NR_FREE_PAGES, -nr_pages);
> +		if (mt == MIGRATE_CMA)

is_migrate_cma()

> +			__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +					      -nr_pages);
>  	}
>  
>  	spin_unlock_irqrestore(&zone->lock, flags);
> @@ -102,6 +106,9 @@ void unset_migratetype_isolate(struct page *page, unsigned migratetype)
>  		goto out;
>  	nr_pages = move_freepages_block(zone, page, migratetype);
>  	__mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
> +	if (migratetype == MIGRATE_CMA)

is_migrate_cma()

> +		__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> +				      nr_pages);
>  	restore_pageblock_isolate(page, migratetype);
>  out:
>  	spin_unlock_irqrestore(&zone->lock, flags);

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]