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

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

[Gioh Kim]

The previous PATCH inserts invalidate_bh_lrus() only into CMA code.
HOTPLUG needs also dropping bh of lru.
So v2 inserts invalidate_bh_lrus() into both of CMA and HOTPLUG.


---------------------------- 8< ----------------------------
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 sometimes costs more than invalidating entire lru.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
 mm/memory_hotplug.c |    1 +
 mm/page_alloc.c     |    2 ++
 2 files changed, 3 insertions(+)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a3797d3..1c5454f 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1672,6 +1672,7 @@ repeat:
                lru_add_drain_all();
                cond_resched();
                drain_all_pages();
+               invalidate_bh_lrus();
        }

        pfn = scan_movable_pages(start_pfn, end_pfn);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b99643d4..c00dedf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
        if (ret)
                return ret;

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andrew Morton]

On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <gioh.kim@lge.com> wrote:

> The previous PATCH inserts invalidate_bh_lrus() only into CMA code.
> HOTPLUG needs also dropping bh of lru.
> So v2 inserts invalidate_bh_lrus() into both of CMA and HOTPLUG.
> 
> 
> ---------------------------- 8< ----------------------------
> 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 sometimes costs more than invalidating entire lru.
> 
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
>  mm/memory_hotplug.c |    1 +
>  mm/page_alloc.c     |    2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index a3797d3..1c5454f 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1672,6 +1672,7 @@ repeat:
>                 lru_add_drain_all();
>                 cond_resched();
>                 drain_all_pages();
> +               invalidate_bh_lrus();

Both of these calls should have a comment explaining why
invalidate_bh_lrus() is being called.

>         }
> 
>         pfn = scan_movable_pages(start_pfn, end_pfn);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b99643d4..c00dedf 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>         if (ret)
>                 return ret;
> 
> +       invalidate_bh_lrus();
> +
>         ret = __alloc_contig_migrate_range(&cc, start, end);
>         if (ret)
>                 goto done;

I do feel that this change is likely to be beneficial, but I don't want
to apply such a patch until I know what its effects are upon all
alloc_contig_range() callers.  Especially hugetlb.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Gioh Kim]

2014-08-01 i??i ? 7:57, Andrew Morton i?' e,?:
> On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <gioh.kim@lge.com> wrote:
>
>> The previous PATCH inserts invalidate_bh_lrus() only into CMA code.
>> HOTPLUG needs also dropping bh of lru.
>> So v2 inserts invalidate_bh_lrus() into both of CMA and HOTPLUG.
>>
>>
>> ---------------------------- 8< ----------------------------
>> 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 sometimes costs more than invalidating entire lru.
>>
>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>> Acked-by: Michal Nazarewicz <mina86@mina86.com>
>> ---
>>   mm/memory_hotplug.c |    1 +
>>   mm/page_alloc.c     |    2 ++
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index a3797d3..1c5454f 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1672,6 +1672,7 @@ repeat:
>>                  lru_add_drain_all();
>>                  cond_resched();
>>                  drain_all_pages();
>> +               invalidate_bh_lrus();
>
> Both of these calls should have a comment explaining why
> invalidate_bh_lrus() is being called.
>
>>          }
>>
>>          pfn = scan_movable_pages(start_pfn, end_pfn);
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index b99643d4..c00dedf 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>>          if (ret)
>>                  return ret;
>>
>> +       invalidate_bh_lrus();
>> +
>>          ret = __alloc_contig_migrate_range(&cc, start, end);
>>          if (ret)
>>                  goto done;
>
> I do feel that this change is likely to be beneficial, but I don't want
> to apply such a patch until I know what its effects are upon all
> alloc_contig_range() callers.  Especially hugetlb.

I'm very sorry to hear that.
How can I check the effects?


> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Joonsoo Kim]

On Fri, Aug 01, 2014 at 09:12:06AM +0900, Gioh Kim wrote:
> 
> 
> 2014-08-01 i??i ? 7:57, Andrew Morton i?' e,?:
> >On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <gioh.kim@lge.com> wrote:
> >
> >>The previous PATCH inserts invalidate_bh_lrus() only into CMA code.
> >>HOTPLUG needs also dropping bh of lru.
> >>So v2 inserts invalidate_bh_lrus() into both of CMA and HOTPLUG.
> >>
> >>
> >>---------------------------- 8< ----------------------------
> >>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 sometimes costs more than invalidating entire lru.
> >>
> >>Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> >>Acked-by: Michal Nazarewicz <mina86@mina86.com>
> >>---
> >>  mm/memory_hotplug.c |    1 +
> >>  mm/page_alloc.c     |    2 ++
> >>  2 files changed, 3 insertions(+)
> >>
> >>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> >>index a3797d3..1c5454f 100644
> >>--- a/mm/memory_hotplug.c
> >>+++ b/mm/memory_hotplug.c
> >>@@ -1672,6 +1672,7 @@ repeat:
> >>                 lru_add_drain_all();
> >>                 cond_resched();
> >>                 drain_all_pages();
> >>+               invalidate_bh_lrus();
> >
> >Both of these calls should have a comment explaining why
> >invalidate_bh_lrus() is being called.
> >
> >>         }
> >>
> >>         pfn = scan_movable_pages(start_pfn, end_pfn);
> >>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >>index b99643d4..c00dedf 100644
> >>--- a/mm/page_alloc.c
> >>+++ b/mm/page_alloc.c
> >>@@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> >>         if (ret)
> >>                 return ret;
> >>
> >>+       invalidate_bh_lrus();
> >>+
> >>         ret = __alloc_contig_migrate_range(&cc, start, end);
> >>         if (ret)
> >>                 goto done;
> >
> >I do feel that this change is likely to be beneficial, but I don't want
> >to apply such a patch until I know what its effects are upon all
> >alloc_contig_range() callers.  Especially hugetlb.
> 
> I'm very sorry to hear that.
> How can I check the effects?
> 

Hello, Gioh.

As you know, I generally agree this patch, but, I want to know that
this patch really fixes your problem. There is some time difference
between invalidate_bh_lrus() and migrate_page() so that the bh of the
migrating page could be re-installed on bh lru again. Any
remarkable success rate changes? If this time gap is critical, we
should put this invalidation logic on other place.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Gioh Kim]

I removed checking migratetype of v1: https://lkml.org/lkml/2014/7/18/82.
Thanks a lot.


---------------------------- 8< ------------------------------