On Tue, May 12, 2015 at 09:51:56AM +0200, Vlastimil Babka wrote:
> On 04/27/2015 09:23 AM, Joonsoo Kim wrote:
> >When we steal whole pageblock, we don't need to break highest order
> >freepage. Perhaps, there is small order freepage so we can use it.
> >
> >This also gives us some code size reduction because expand() which
> >is used in __rmqueue_fallback() and inlined into __rmqueue_fallback()
> >is removed.
> >
> > text data bss dec hex filename
> > 37413 1440 624 39477 9a35 mm/page_alloc.o
> > 37249 1440 624 39313 9991 mm/page_alloc.o
> >
> >Signed-off-by: Joonsoo Kim <iamjoonsoo....@lge.com>
> >---
> > mm/page_alloc.c | 40 +++++++++++++++++++++-------------------
> > 1 file changed, 21 insertions(+), 19 deletions(-)
> >
> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >index ed0f1c6..044f16c 100644
> >--- a/mm/page_alloc.c
> >+++ b/mm/page_alloc.c
> >@@ -1239,14 +1239,14 @@ int find_suitable_fallback(struct free_area *area,
> >unsigned int order,
> > }
> >
> > /* Remove an element from the buddy allocator from the fallback list */
>
> This is no longer accurate description.
Okay. Will fix.
>
> >-static inline struct page *
> >+static inline bool
> > __rmqueue_fallback(struct zone *zone, unsigned int order, int
> > start_migratetype)
> > {
> > struct free_area *area;
> > unsigned int current_order;
> > struct page *page;
> > int fallback_mt;
> >- bool can_steal;
> >+ bool can_steal_pageblock;
> >
> > /* Find the largest possible block of pages in the other list */
> > for (current_order = MAX_ORDER-1;
> >@@ -1254,26 +1254,24 @@ __rmqueue_fallback(struct zone *zone, unsigned int
> >order, int start_migratetype)
> > --current_order) {
> > area = &(zone->free_area[current_order]);
> > fallback_mt = find_suitable_fallback(area, current_order,
> >- start_migratetype, false, &can_steal);
> >+ start_migratetype, false,
> >+ &can_steal_pageblock);
> > if (fallback_mt == -1)
> > continue;
> >
> > page = list_entry(area->free_list[fallback_mt].next,
> > struct page, lru);
>
> >- if (can_steal)
> >+ BUG_ON(!page);
>
> Please no new BUG_ON. VM_BUG_ON maybe for debugging, otherwise just
> let it panic on null pointer exception accessing page->lru later on.
Okay. I will remove it.
> >+
> >+ if (can_steal_pageblock)
> > steal_suitable_fallback(zone, page, start_migratetype);
> >
> >- /* Remove the page from the freelists */
> >- area->nr_free--;
> >- list_del(&page->lru);
> >- rmv_page_order(page);
> >+ list_move(&page->lru, &area->free_list[start_migratetype]);
>
> This list_move is redundant if we are stealing whole pageblock,
> right? Just put it in an else of the if above, and explain in
> comment?
I tried to put list_move() in an else of the if above and I got a
panic problem due to failure of stealing whole pageblock. In that
time, I didn't want to deep dive into the problem so used a simple
solution like as above. But, now I find the reason why steal sometimes
fail so I can fix it. I will fix it on next spin.
>
> >- expand(zone, page, order, current_order, area,
> >- start_migratetype);
> > /*
> > * The freepage_migratetype may differ from pageblock's
> > * migratetype depending on the decisions in
> >- * try_to_steal_freepages(). This is OK as long as it
> >+ * find_suitable_fallback(). This is OK as long as it
> > * does not differ for MIGRATE_CMA pageblocks. For CMA
> > * we need to make sure unallocated pages flushed from
> > * pcp lists are returned to the correct freelist.
>
> The whole thing with set_freepage_migratetype(page,
> start_migratetype); below this comment is now redundant, as
> rmqueue_smallest will do it too.
> The comment itself became outdated and misplaced too. I guess
> MIGRATE_CMA is now handled just by the fact that is is not set as
> fallback in the fallbacks array?
Okay. All comment seems to be redundant. I will remove it.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
