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

PATCH In review Gioh Kim 3 participants 4 messages Jul 31, 2014 – Aug 6, 2014

Subsystems (3)

Memory management Page allocator Memory hot(un)plug

lore

Share & tools

lore

Gioh Kim

author

2014-07-31 02:22 UTC #0 lore raw

PATCH v2 1/1 CMA/HOTPLUG: clear buffer-head lru before page migration

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 <[email protected]>

Acked-by Michal Nazarewicz <[email protected]>

---
 mm/memory_hotplug.c |    1 +
 mm/page_alloc.c     |    2 ++
 2 files changed, 3 insertions(+)

mm/memory_hotplug.c +1 -0

index a3797d3..1c5454f 100644
@@ -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);

mm/page_alloc.c +2 -0

index b99643d4..c00dedf 100644
@@ -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

Andrew Morton

maintainer

2014-07-31 22:57 UTC #1 lore raw

Quoted 25 lines · On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <[email protected]> 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 <[email protected]>
Acked-by Michal Nazarewicz <[email protected]>
---
 mm/memory_hotplug.c |    1 +
 mm/page_alloc.c     |    2 ++
 2 files changed, 3 insertions(+)
mm/memory_hotplug.c +0 -0
index a3797d3..1c5454f 100644

mm/memory_hotplug.c repeat: · line 1675

Quoted 4 lines

mm/memory_hotplug.c +1 -0

@@ -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.

Quoted 8 lines

mm/memory_hotplug.c +0 -0

        }
 
        pfn = scan_movable_pages(start_pfn, end_pfn);

mm/page_alloc.c +0 -0

index b99643d4..c00dedf 100644

mm/page_alloc.c int alloc_contig_range(unsigned long start, unsigned long end, · line 6376

Quoted 8 lines

mm/page_alloc.c +2 -0

@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Gioh Kim

author

2014-08-01 00:12 UTC #2 lore raw

2014-08-01 i??i ? 7:57, Andrew Morton i?' e,?:

Quoted 46 lines · On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <[email protected]> wrote:

Quoted 26 lines
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 <[email protected]>
Acked-by Michal Nazarewicz <[email protected]>
---
  mm/memory_hotplug.c |    1 +
  mm/page_alloc.c     |    2 ++
  2 files changed, 3 insertions(+)
mm/memory_hotplug.c +0 -0
index a3797d3..1c5454f 100644
mm/memory_hotplug.c repeat: · line 1675
Quoted 4 lines
mm/memory_hotplug.c +1 -0
@@ -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.
Quoted 11 lines
mm/memory_hotplug.c +0 -0
         }
 
         pfn = scan_movable_pages(start_pfn, end_pfn);
mm/page_alloc.c +0 -0
index b99643d4..c00dedf 100644
@@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
         if (ret)
                 return ret;
 

Quoted 9 lines

mm/page_alloc.c int alloc_contig_range(unsigned long start, unsigned long end, · line 6376
Quoted 5 lines
mm/page_alloc.c +2 -0
+       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?

Quoted 6 lines

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

Joonsoo Kim

2014-08-06 07:30 UTC #3 lore raw

Quoted 59 lines · Gioh Kim

2014-08-01 i??i ? 7:57, Andrew Morton i?' e,?:
Quoted 46 lines · On Thu, 31 Jul 2014 11:22:35 +0900 Gioh Kim <[email protected]> wrote:
Quoted 26 lines
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 <[email protected]>
Acked-by Michal Nazarewicz <[email protected]>
---
 mm/memory_hotplug.c |    1 +
 mm/page_alloc.c     |    2 ++
 2 files changed, 3 insertions(+)
mm/memory_hotplug.c +0 -0
index a3797d3..1c5454f 100644
mm/memory_hotplug.c repeat: · line 1675
Quoted 4 lines
mm/memory_hotplug.c +1 -0
@@ -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.
Quoted 11 lines
mm/memory_hotplug.c +0 -0
        }
 
        pfn = scan_movable_pages(start_pfn, end_pfn);
mm/page_alloc.c +0 -0
index b99643d4..c00dedf 100644
@@ -6369,6 +6369,8 @@ int alloc_contig_range(unsigned long start, unsigned long end,
        if (ret)
                return ret;
 
Quoted 9 lines
mm/page_alloc.c int alloc_contig_range(unsigned long start, unsigned long end, · line 6376
Quoted 5 lines
mm/page_alloc.c +2 -0
+       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.

Quoted 3 lines

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.