Re: [PATCH] CMA/HOTPLUG: clear buffer-head lru before page migration

[Michal Nazarewicz]

>> On 2014-07-18 08:45, Gioh Kim wrote:
>>> From: Gioh Kim <gioh.kim@xxxxxxx>
>>> Date: Fri, 18 Jul 2014 13:40:01 +0900
>>> Subject: [PATCH] CMA/HOTPLUG: clear buffer-head lru before page migration
>>>
>>> The bh must be free to migrate a page at which bh is mapped.
>>> The reference count of bh is increased when it is installed
>>> into lru so that the bh of lru must be freed before migrating the page.
>>>
>>> This frees every bh of lru. We could free only bh of migrating page.
>>> But searching lru costs more than invalidating entire lru.
>>>
>>> Signed-off-by: Gioh Kim <gioh.kim@xxxxxxx>
>>> Acked-by: Laura Abbott <lauraa@xxxxxxxxxxxxxx>

With the if removed:

Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>

>>> ---
>>>   mm/page_alloc.c |    3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index b99643d4..3b474e0 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -6369,6 +6369,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>>>          if (ret)
>>>                  return ret;
>>>
>>> +       if (migratetype == MIGRATE_CMA || migratetype == MIGRATE_MOVABLE)

On Fri, Jul 18 2014, Gioh Kim <gioh.kim@xxxxxxx> wrote:
> I agree. I cannot understand why alloc_contig_range has an argument of
> migratetype.  Can the alloc_contig_range is called for other migrate
> type than CMA/MOVABLE?

It has migratetype argument precisely because it can be CMA or MOVABLE.
If alloc_contig_range was called always with the same migrate type, the
argument would not be necessary, but because it isn't, it is.

> What do you think about removing the argument of migratetype and
> checking migratetype (if (migratetype == MIGRATE_CMA || migratetype ==
> MIGRATE_MOVABLE))?

If you remove the argument, the function would have to read migrate type
of the pageblock and that's just waste of time, since the migrate type
can be passed to the function from its caller, so the argument should
remain.

>>> +               invalidate_bh_lrus();
>>> +
>>>          ret = __alloc_contig_migrate_range(&cc, start, end);
>>>          if (ret)
>>>                  goto done;

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  MichaÅ âmina86â Nazarewicz    (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/