Re: [RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Fri, Jul 27, 2012 at 07:22:48PM +0900, Kamezawa Hiroyuki wrote:
> (2012/07/23 9:48), Minchan Kim wrote:
> > Like below, memory-hotplug makes race between page-isolation
> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
> >
> > CPU A CPU B
> >
> > start_isolate_page_range
> > set_migratetype_isolate
> > spin_lock_irqsave(zone->lock)
> >
> > free_hot_cold_page(Page A)
> > /* without zone->lock */
> > migratetype = get_pageblock_migratetype(Page A);
> > /*
> > * Page could be moved into MIGRATE_MOVABLE
> > * of per_cpu_pages
> > */
> > list_add_tail(>lru,
> > >lists[migratetype]);
> >
> > set_pageblock_isolate
> > move_freepages_block
> > drain_all_pages
> >
> > /* Page A could be in MIGRATE_MOVABLE of
> > free_list. */
> >
> > check_pages_isolated
> > __test_page_isolated_in_pageblock
> > /*
> > * We can't catch freed page which
> > * is free_list[MIGRATE_MOVABLE]
> > */
> > if (PageBuddy(page A))
> > pfn += 1 << page_order(page A);
> >
> > /* So, Page A could be allocated */
> >
> > __offline_isolated_pages
> > /*
> > * BUG_ON hit or offline page
> > * which is used by someone
> > */
> > BUG_ON(!PageBuddy(page A));
> >
> > Signed-off-by: Minchan Kim
>
> Ah, hm. Then, you say the page in MIGRATE_MOVABLE will not be isolated
> and may be used again.
Yes.
>
>
> > ---
> > I found this problem during code review so please confirm it.
> > Kame?
> >
> > mm/page_isolation.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> > index acf65a7..4699d1f 100644
> > --- a/mm/page_isolation.c
> > +++ b/mm/page_isolation.c
> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
> > unsigned long end_pfn)
> > continue;
> > }
> > page = pfn_to_page(pfn);
> > - if (PageBuddy(page))
> > + if (PageBuddy(page)) {
> > + if (get_page_migratetype(page) != MIGRATE_ISOLATE)
> > + break;
>
> Doesn't this work enough ? The problem is MIGRATE_TYPE and list_head
> mis-match.
I guess you are confused between get_page_migratetype and
get_pageblock_migratetype.
It's not get_pageblock_migratetype but get_page_migratetype which is introduced
for detecting
MIGRATE_TYPE and list_head mismatch in [1,2/3].
>
> Thanks,
> -Kame
>
>
>
--
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/
Re: [RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
(2012/07/23 9:48), Minchan Kim wrote:
> Like below, memory-hotplug makes race between page-isolation
> and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
>
> CPU A CPU B
>
> start_isolate_page_range
> set_migratetype_isolate
> spin_lock_irqsave(zone->lock)
>
> free_hot_cold_page(Page A)
> /* without zone->lock */
> migratetype = get_pageblock_migratetype(Page A);
> /*
>* Page could be moved into MIGRATE_MOVABLE
>* of per_cpu_pages
>*/
> list_add_tail(>lru,
> >lists[migratetype]);
>
> set_pageblock_isolate
> move_freepages_block
> drain_all_pages
>
> /* Page A could be in MIGRATE_MOVABLE of
> free_list. */
>
> check_pages_isolated
> __test_page_isolated_in_pageblock
> /*
> * We can't catch freed page which
> * is free_list[MIGRATE_MOVABLE]
> */
> if (PageBuddy(page A))
> pfn += 1 << page_order(page A);
>
> /* So, Page A could be allocated */
>
> __offline_isolated_pages
> /*
> * BUG_ON hit or offline page
> * which is used by someone
> */
> BUG_ON(!PageBuddy(page A));
>
> Signed-off-by: Minchan Kim
Ah, hm. Then, you say the page in MIGRATE_MOVABLE will not be isolated
and may be used again.
> ---
> I found this problem during code review so please confirm it.
> Kame?
>
> mm/page_isolation.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index acf65a7..4699d1f 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
> unsigned long end_pfn)
> continue;
> }
> page = pfn_to_page(pfn);
> - if (PageBuddy(page))
> + if (PageBuddy(page)) {
> + if (get_page_migratetype(page) != MIGRATE_ISOLATE)
> + break;
Doesn't this work enough ? The problem is MIGRATE_TYPE and list_head mis-match.
Thanks,
-Kame
--
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/
Re: [RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
(2012/07/23 9:48), Minchan Kim wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
Ah, hm. Then, you say the page in MIGRATE_MOVABLE will not be isolated
and may be used again.
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
Doesn't this work enough ? The problem is MIGRATE_TYPE and list_head mis-match.
Thanks,
-Kame
--
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/
Re: [RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Fri, Jul 27, 2012 at 07:22:48PM +0900, Kamezawa Hiroyuki wrote:
(2012/07/23 9:48), Minchan Kim wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
Ah, hm. Then, you say the page in MIGRATE_MOVABLE will not be isolated
and may be used again.
Yes.
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
Doesn't this work enough ? The problem is MIGRATE_TYPE and list_head
mis-match.
I guess you are confused between get_page_migratetype and
get_pageblock_migratetype.
It's not get_pageblock_migratetype but get_page_migratetype which is introduced
for detecting
MIGRATE_TYPE and list_head mismatch in [1,2/3].
Thanks,
-Kame
--
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/
[RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone->lock)
free_hot_cold_page(Page A)
/* without zone->lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(>lru,
>lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 << page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
pfn += 1 << page_order(page);
+ }
else if (page_count(page) == 0 &&
get_page_migratetype(page) == MIGRATE_ISOLATE)
pfn += 1;
--
1.7.9.5
--
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/
[RESEND RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
pfn += 1 page_order(page);
+ }
else if (page_count(page) == 0
get_page_migratetype(page) == MIGRATE_ISOLATE)
pfn += 1;
--
1.7.9.5
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 01:29:47PM +0800, Bob Liu wrote:
> On Wed, Jul 18, 2012 at 11:56 AM, Minchan Kim wrote:
> > On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
> >> On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim wrote:
> >> > On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
> >> >> On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
> >> >> > Hi Bob,
> >> >> >
> >> >> > On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
> >> >> >> Hi Minchan,
> >> >> >>
> >> >> >> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim
> >> >> >> wrote:
> >> >> >> > Like below, memory-hotplug makes race between page-isolation
> >> >> >> > and page-allocation so it can hit BUG_ON in
> >> >> >> > __offline_isolated_pages.
> >> >> >> >
> >> >> >> > CPU A CPU B
> >> >> >> >
> >> >> >> > start_isolate_page_range
> >> >> >> > set_migratetype_isolate
> >> >> >> > spin_lock_irqsave(zone->lock)
> >> >> >> >
> >> >> >> > free_hot_cold_page(Page A)
> >> >> >> > /* without zone->lock */
> >> >> >> > migratetype =
> >> >> >> > get_pageblock_migratetype(Page A);
> >> >> >> > /*
> >> >> >> > * Page could be moved into
> >> >> >> > MIGRATE_MOVABLE
> >> >> >> > * of per_cpu_pages
> >> >> >> > */
> >> >> >> > list_add_tail(>lru,
> >> >> >> > >lists[migratetype]);
> >> >> >> >
> >> >> >> > set_pageblock_isolate
> >> >> >> > move_freepages_block
> >> >> >> > drain_all_pages
> >> >> >> >
> >> >> >> > /* Page A could be in
> >> >> >> > MIGRATE_MOVABLE of free_list. */
> >> >> >> >
> >> >> >> > check_pages_isolated
> >> >> >> > __test_page_isolated_in_pageblock
> >> >> >> > /*
> >> >> >> > * We can't catch freed page which
> >> >> >> > * is free_list[MIGRATE_MOVABLE]
> >> >> >> > */
> >> >> >> > if (PageBuddy(page A))
> >> >> >> > pfn += 1 << page_order(page A);
> >> >> >> >
> >> >> >> > /* So, Page A could be allocated */
> >> >> >> >
> >> >> >> > __offline_isolated_pages
> >> >> >> > /*
> >> >> >> > * BUG_ON hit or offline page
> >> >> >> > * which is used by someone
> >> >> >> > */
> >> >> >> > BUG_ON(!PageBuddy(page A));
> >> >> >> >
> >> >> >> > Signed-off-by: Minchan Kim
> >> >> >> > ---
> >> >> >> > I found this problem during code review so please confirm it.
> >> >> >> > Kame?
> >> >> >> >
> >> >> >> > mm/page_isolation.c |5 -
> >> >> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >> >> >> >
> >> >> >> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> >> >> >> > index acf65a7..4699d1f 100644
> >> >> >> > --- a/mm/page_isolation.c
> >> >> >> > +++ b/mm/page_isolation.c
> >> >> >> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned
> >> >> >> > long pfn, unsigned long end_pfn)
> >> >> >> > continue;
> >> >> >> > }
> >> >> >> > page = pfn_to_page(pfn);
> >> >> >> > - if (PageBuddy(page))
> >> >> >> > + if (PageBuddy(page)) {
> >> >> >> > pfn += 1 << page_order(page);
> >> >> >> > + if (get_page_migratetype(page) !=
> >> >> >> > MIGRATE_ISOLATE)
> >> >> >> > + break;
> >> >> >> > + }
> >> >> >>
> >> >> >> test_page_isolated() already have check
> >> >> >> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
> >> >> >>
> >> >> >
> >> >> > That's why I send a patch.
> >> >> > As I describe in description, pageblock migration type of
> >> >> > get_page_migratetype(page)
> >> >> > is inconsistent with free_list[migrationtype].
> >> >> > I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE
> >> >> > but the page would be
> >> >> > in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
> >> >> > that race happens.
> >> >> >
> >> >>
> >> >> Sorry, I'm still not get the situation how this race happens.
> >> >>
> >> >> set_pageblock_isolate
> >> >> move_freepages_block
> >> >> drain_all_pages
> >> >>
> >> >> /* Page A could be in MIGRATE_MOVABLE
> >> >> of free_list. */
> >> >>
> >> >> I think move_freepages_block() will call list_move() to move Page A to
> >> >> free_list[MIGRATE_ISOLATE], so this case can't happen?
> >> >
> >> > move_freepages_block handles only pages in free_list but Page A is on
> >> > pcp, not free_list.
> >> >
> >>
> >> Got it, then why not just drain pcp pages before move_freepages_block() ?
> >
> > CPU A CPU B
> >
> > drain_all_pages
> > lock(zone->lock);
> > free_hot_cold_page
> > MIGRATE_MOVABLE =
> > get_pageblock_migratetype(page);
> >
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 11:56 AM, Minchan Kim wrote:
> On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
>> On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim wrote:
>> > On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
>> >> On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
>> >> > Hi Bob,
>> >> >
>> >> > On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
>> >> >> Hi Minchan,
>> >> >>
>> >> >> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim
>> >> >> wrote:
>> >> >> > Like below, memory-hotplug makes race between page-isolation
>> >> >> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
>> >> >> >
>> >> >> > CPU A CPU B
>> >> >> >
>> >> >> > start_isolate_page_range
>> >> >> > set_migratetype_isolate
>> >> >> > spin_lock_irqsave(zone->lock)
>> >> >> >
>> >> >> > free_hot_cold_page(Page A)
>> >> >> > /* without zone->lock */
>> >> >> > migratetype =
>> >> >> > get_pageblock_migratetype(Page A);
>> >> >> > /*
>> >> >> > * Page could be moved into
>> >> >> > MIGRATE_MOVABLE
>> >> >> > * of per_cpu_pages
>> >> >> > */
>> >> >> > list_add_tail(>lru,
>> >> >> > >lists[migratetype]);
>> >> >> >
>> >> >> > set_pageblock_isolate
>> >> >> > move_freepages_block
>> >> >> > drain_all_pages
>> >> >> >
>> >> >> > /* Page A could be in
>> >> >> > MIGRATE_MOVABLE of free_list. */
>> >> >> >
>> >> >> > check_pages_isolated
>> >> >> > __test_page_isolated_in_pageblock
>> >> >> > /*
>> >> >> > * We can't catch freed page which
>> >> >> > * is free_list[MIGRATE_MOVABLE]
>> >> >> > */
>> >> >> > if (PageBuddy(page A))
>> >> >> > pfn += 1 << page_order(page A);
>> >> >> >
>> >> >> > /* So, Page A could be allocated */
>> >> >> >
>> >> >> > __offline_isolated_pages
>> >> >> > /*
>> >> >> > * BUG_ON hit or offline page
>> >> >> > * which is used by someone
>> >> >> > */
>> >> >> > BUG_ON(!PageBuddy(page A));
>> >> >> >
>> >> >> > Signed-off-by: Minchan Kim
>> >> >> > ---
>> >> >> > I found this problem during code review so please confirm it.
>> >> >> > Kame?
>> >> >> >
>> >> >> > mm/page_isolation.c |5 -
>> >> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >> >
>> >> >> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> >> >> > index acf65a7..4699d1f 100644
>> >> >> > --- a/mm/page_isolation.c
>> >> >> > +++ b/mm/page_isolation.c
>> >> >> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
>> >> >> > pfn, unsigned long end_pfn)
>> >> >> > continue;
>> >> >> > }
>> >> >> > page = pfn_to_page(pfn);
>> >> >> > - if (PageBuddy(page))
>> >> >> > + if (PageBuddy(page)) {
>> >> >> > pfn += 1 << page_order(page);
>> >> >> > + if (get_page_migratetype(page) !=
>> >> >> > MIGRATE_ISOLATE)
>> >> >> > + break;
>> >> >> > + }
>> >> >>
>> >> >> test_page_isolated() already have check
>> >> >> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
>> >> >>
>> >> >
>> >> > That's why I send a patch.
>> >> > As I describe in description, pageblock migration type of
>> >> > get_page_migratetype(page)
>> >> > is inconsistent with free_list[migrationtype].
>> >> > I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but
>> >> > the page would be
>> >> > in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
>> >> > that race happens.
>> >> >
>> >>
>> >> Sorry, I'm still not get the situation how this race happens.
>> >>
>> >> set_pageblock_isolate
>> >> move_freepages_block
>> >> drain_all_pages
>> >>
>> >> /* Page A could be in MIGRATE_MOVABLE
>> >> of free_list. */
>> >>
>> >> I think move_freepages_block() will call list_move() to move Page A to
>> >> free_list[MIGRATE_ISOLATE], so this case can't happen?
>> >
>> > move_freepages_block handles only pages in free_list but Page A is on pcp,
>> > not free_list.
>> >
>>
>> Got it, then why not just drain pcp pages before move_freepages_block() ?
>
> CPU A CPU B
>
> drain_all_pages
> lock(zone->lock);
> free_hot_cold_page
> MIGRATE_MOVABLE =
> get_pageblock_migratetype(page);
> list_add(>lru, >lists[migratetype])
> set_pageblock_isolate
> move_free_pages_block
> unlock(zone->lock);
>
> We can't make it atomic.
>
>>
>> And I didn't see the effect by adding the check if
>> (get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
>> Since set_pageblock_isolate() have been
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
> On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim wrote:
> > On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
> >> On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
> >> > Hi Bob,
> >> >
> >> > On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
> >> >> Hi Minchan,
> >> >>
> >> >> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
> >> >> > Like below, memory-hotplug makes race between page-isolation
> >> >> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
> >> >> >
> >> >> > CPU A CPU B
> >> >> >
> >> >> > start_isolate_page_range
> >> >> > set_migratetype_isolate
> >> >> > spin_lock_irqsave(zone->lock)
> >> >> >
> >> >> > free_hot_cold_page(Page A)
> >> >> > /* without zone->lock */
> >> >> > migratetype =
> >> >> > get_pageblock_migratetype(Page A);
> >> >> > /*
> >> >> > * Page could be moved into
> >> >> > MIGRATE_MOVABLE
> >> >> > * of per_cpu_pages
> >> >> > */
> >> >> > list_add_tail(>lru,
> >> >> > >lists[migratetype]);
> >> >> >
> >> >> > set_pageblock_isolate
> >> >> > move_freepages_block
> >> >> > drain_all_pages
> >> >> >
> >> >> > /* Page A could be in MIGRATE_MOVABLE
> >> >> > of free_list. */
> >> >> >
> >> >> > check_pages_isolated
> >> >> > __test_page_isolated_in_pageblock
> >> >> > /*
> >> >> > * We can't catch freed page which
> >> >> > * is free_list[MIGRATE_MOVABLE]
> >> >> > */
> >> >> > if (PageBuddy(page A))
> >> >> > pfn += 1 << page_order(page A);
> >> >> >
> >> >> > /* So, Page A could be allocated */
> >> >> >
> >> >> > __offline_isolated_pages
> >> >> > /*
> >> >> > * BUG_ON hit or offline page
> >> >> > * which is used by someone
> >> >> > */
> >> >> > BUG_ON(!PageBuddy(page A));
> >> >> >
> >> >> > Signed-off-by: Minchan Kim
> >> >> > ---
> >> >> > I found this problem during code review so please confirm it.
> >> >> > Kame?
> >> >> >
> >> >> > mm/page_isolation.c |5 -
> >> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> >> >> > index acf65a7..4699d1f 100644
> >> >> > --- a/mm/page_isolation.c
> >> >> > +++ b/mm/page_isolation.c
> >> >> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
> >> >> > pfn, unsigned long end_pfn)
> >> >> > continue;
> >> >> > }
> >> >> > page = pfn_to_page(pfn);
> >> >> > - if (PageBuddy(page))
> >> >> > + if (PageBuddy(page)) {
> >> >> > pfn += 1 << page_order(page);
> >> >> > + if (get_page_migratetype(page) !=
> >> >> > MIGRATE_ISOLATE)
> >> >> > + break;
> >> >> > + }
> >> >>
> >> >> test_page_isolated() already have check
> >> >> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
> >> >>
> >> >
> >> > That's why I send a patch.
> >> > As I describe in description, pageblock migration type of
> >> > get_page_migratetype(page)
> >> > is inconsistent with free_list[migrationtype].
> >> > I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but
> >> > the page would be
> >> > in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
> >> > that race happens.
> >> >
> >>
> >> Sorry, I'm still not get the situation how this race happens.
> >>
> >> set_pageblock_isolate
> >> move_freepages_block
> >> drain_all_pages
> >>
> >> /* Page A could be in MIGRATE_MOVABLE
> >> of free_list. */
> >>
> >> I think move_freepages_block() will call list_move() to move Page A to
> >> free_list[MIGRATE_ISOLATE], so this case can't happen?
> >
> > move_freepages_block handles only pages in free_list but Page A is on pcp,
> > not free_list.
> >
>
> Got it, then why not just drain pcp pages before move_freepages_block() ?
CPU A CPU B
drain_all_pages
lock(zone->lock);
free_hot_cold_page
MIGRATE_MOVABLE =
get_pageblock_migratetype(page);
list_add(>lru, >lists[migratetype])
set_pageblock_isolate
move_free_pages_block
unlock(zone->lock);
We can't make it atomic.
>
> And I didn't see the effect by adding the check if
> (get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
> Since set_pageblock_isolate() have been called by CPU A, this check
> will always false which cause CPU A still consider Page A isolated,
> then PAGE A still can be allocated by CPU B from pcp.
Please don't confuse get_page_migratetype and
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim wrote:
> On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
>> On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
>> > Hi Bob,
>> >
>> > On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
>> >> Hi Minchan,
>> >>
>> >> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
>> >> > Like below, memory-hotplug makes race between page-isolation
>> >> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
>> >> >
>> >> > CPU A CPU B
>> >> >
>> >> > start_isolate_page_range
>> >> > set_migratetype_isolate
>> >> > spin_lock_irqsave(zone->lock)
>> >> >
>> >> > free_hot_cold_page(Page A)
>> >> > /* without zone->lock */
>> >> > migratetype =
>> >> > get_pageblock_migratetype(Page A);
>> >> > /*
>> >> > * Page could be moved into
>> >> > MIGRATE_MOVABLE
>> >> > * of per_cpu_pages
>> >> > */
>> >> > list_add_tail(>lru,
>> >> > >lists[migratetype]);
>> >> >
>> >> > set_pageblock_isolate
>> >> > move_freepages_block
>> >> > drain_all_pages
>> >> >
>> >> > /* Page A could be in MIGRATE_MOVABLE
>> >> > of free_list. */
>> >> >
>> >> > check_pages_isolated
>> >> > __test_page_isolated_in_pageblock
>> >> > /*
>> >> > * We can't catch freed page which
>> >> > * is free_list[MIGRATE_MOVABLE]
>> >> > */
>> >> > if (PageBuddy(page A))
>> >> > pfn += 1 << page_order(page A);
>> >> >
>> >> > /* So, Page A could be allocated */
>> >> >
>> >> > __offline_isolated_pages
>> >> > /*
>> >> > * BUG_ON hit or offline page
>> >> > * which is used by someone
>> >> > */
>> >> > BUG_ON(!PageBuddy(page A));
>> >> >
>> >> > Signed-off-by: Minchan Kim
>> >> > ---
>> >> > I found this problem during code review so please confirm it.
>> >> > Kame?
>> >> >
>> >> > mm/page_isolation.c |5 -
>> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> >> > index acf65a7..4699d1f 100644
>> >> > --- a/mm/page_isolation.c
>> >> > +++ b/mm/page_isolation.c
>> >> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
>> >> > pfn, unsigned long end_pfn)
>> >> > continue;
>> >> > }
>> >> > page = pfn_to_page(pfn);
>> >> > - if (PageBuddy(page))
>> >> > + if (PageBuddy(page)) {
>> >> > pfn += 1 << page_order(page);
>> >> > + if (get_page_migratetype(page) !=
>> >> > MIGRATE_ISOLATE)
>> >> > + break;
>> >> > + }
>> >>
>> >> test_page_isolated() already have check
>> >> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
>> >>
>> >
>> > That's why I send a patch.
>> > As I describe in description, pageblock migration type of
>> > get_page_migratetype(page)
>> > is inconsistent with free_list[migrationtype].
>> > I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
>> > page would be
>> > in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
>> > race happens.
>> >
>>
>> Sorry, I'm still not get the situation how this race happens.
>>
>> set_pageblock_isolate
>> move_freepages_block
>> drain_all_pages
>>
>> /* Page A could be in MIGRATE_MOVABLE
>> of free_list. */
>>
>> I think move_freepages_block() will call list_move() to move Page A to
>> free_list[MIGRATE_ISOLATE], so this case can't happen?
>
> move_freepages_block handles only pages in free_list but Page A is on pcp,
> not free_list.
>
Got it, then why not just drain pcp pages before move_freepages_block() ?
And I didn't see the effect by adding the check if
(get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
Since set_pageblock_isolate() have been called by CPU A, this check
will always false which cause CPU A still consider Page A isolated,
then PAGE A still can be allocated by CPU B from pcp.
--
Regards,
--Bob
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
> On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
> > Hi Bob,
> >
> > On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
> >> Hi Minchan,
> >>
> >> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
> >> > Like below, memory-hotplug makes race between page-isolation
> >> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
> >> >
> >> > CPU A CPU B
> >> >
> >> > start_isolate_page_range
> >> > set_migratetype_isolate
> >> > spin_lock_irqsave(zone->lock)
> >> >
> >> > free_hot_cold_page(Page A)
> >> > /* without zone->lock */
> >> > migratetype =
> >> > get_pageblock_migratetype(Page A);
> >> > /*
> >> > * Page could be moved into
> >> > MIGRATE_MOVABLE
> >> > * of per_cpu_pages
> >> > */
> >> > list_add_tail(>lru,
> >> > >lists[migratetype]);
> >> >
> >> > set_pageblock_isolate
> >> > move_freepages_block
> >> > drain_all_pages
> >> >
> >> > /* Page A could be in MIGRATE_MOVABLE of
> >> > free_list. */
> >> >
> >> > check_pages_isolated
> >> > __test_page_isolated_in_pageblock
> >> > /*
> >> > * We can't catch freed page which
> >> > * is free_list[MIGRATE_MOVABLE]
> >> > */
> >> > if (PageBuddy(page A))
> >> > pfn += 1 << page_order(page A);
> >> >
> >> > /* So, Page A could be allocated */
> >> >
> >> > __offline_isolated_pages
> >> > /*
> >> > * BUG_ON hit or offline page
> >> > * which is used by someone
> >> > */
> >> > BUG_ON(!PageBuddy(page A));
> >> >
> >> > Signed-off-by: Minchan Kim
> >> > ---
> >> > I found this problem during code review so please confirm it.
> >> > Kame?
> >> >
> >> > mm/page_isolation.c |5 -
> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> >> > index acf65a7..4699d1f 100644
> >> > --- a/mm/page_isolation.c
> >> > +++ b/mm/page_isolation.c
> >> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
> >> > pfn, unsigned long end_pfn)
> >> > continue;
> >> > }
> >> > page = pfn_to_page(pfn);
> >> > - if (PageBuddy(page))
> >> > + if (PageBuddy(page)) {
> >> > pfn += 1 << page_order(page);
> >> > + if (get_page_migratetype(page) !=
> >> > MIGRATE_ISOLATE)
> >> > + break;
> >> > + }
> >>
> >> test_page_isolated() already have check
> >> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
> >>
> >
> > That's why I send a patch.
> > As I describe in description, pageblock migration type of
> > get_page_migratetype(page)
> > is inconsistent with free_list[migrationtype].
> > I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
> > page would be
> > in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
> > race happens.
> >
>
> Sorry, I'm still not get the situation how this race happens.
>
> set_pageblock_isolate
> move_freepages_block
> drain_all_pages
>
> /* Page A could be in MIGRATE_MOVABLE
> of free_list. */
>
> I think move_freepages_block() will call list_move() to move Page A to
> free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on pcp, not
free_list.
>
> --
> Thanks,
> --Bob
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org
--
Kind regards,
Minchan Kim
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim wrote:
> Hi Bob,
>
> On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
>> Hi Minchan,
>>
>> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
>> > Like below, memory-hotplug makes race between page-isolation
>> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
>> >
>> > CPU A CPU B
>> >
>> > start_isolate_page_range
>> > set_migratetype_isolate
>> > spin_lock_irqsave(zone->lock)
>> >
>> > free_hot_cold_page(Page A)
>> > /* without zone->lock */
>> > migratetype =
>> > get_pageblock_migratetype(Page A);
>> > /*
>> > * Page could be moved into MIGRATE_MOVABLE
>> > * of per_cpu_pages
>> > */
>> > list_add_tail(>lru,
>> > >lists[migratetype]);
>> >
>> > set_pageblock_isolate
>> > move_freepages_block
>> > drain_all_pages
>> >
>> > /* Page A could be in MIGRATE_MOVABLE of
>> > free_list. */
>> >
>> > check_pages_isolated
>> > __test_page_isolated_in_pageblock
>> > /*
>> > * We can't catch freed page which
>> > * is free_list[MIGRATE_MOVABLE]
>> > */
>> > if (PageBuddy(page A))
>> > pfn += 1 << page_order(page A);
>> >
>> > /* So, Page A could be allocated */
>> >
>> > __offline_isolated_pages
>> > /*
>> > * BUG_ON hit or offline page
>> > * which is used by someone
>> > */
>> > BUG_ON(!PageBuddy(page A));
>> >
>> > Signed-off-by: Minchan Kim
>> > ---
>> > I found this problem during code review so please confirm it.
>> > Kame?
>> >
>> > mm/page_isolation.c |5 -
>> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> > index acf65a7..4699d1f 100644
>> > --- a/mm/page_isolation.c
>> > +++ b/mm/page_isolation.c
>> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
>> > unsigned long end_pfn)
>> > continue;
>> > }
>> > page = pfn_to_page(pfn);
>> > - if (PageBuddy(page))
>> > + if (PageBuddy(page)) {
>> > pfn += 1 << page_order(page);
>> > + if (get_page_migratetype(page) != MIGRATE_ISOLATE)
>> > + break;
>> > + }
>>
>> test_page_isolated() already have check
>> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
>>
>
> That's why I send a patch.
> As I describe in description, pageblock migration type of
> get_page_migratetype(page)
> is inconsistent with free_list[migrationtype].
> I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
> page would be
> in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
> race happens.
>
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
--
Thanks,
--Bob
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
> Hi Minchan,
>
> On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
> > Like below, memory-hotplug makes race between page-isolation
> > and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
> >
> > CPU A CPU B
> >
> > start_isolate_page_range
> > set_migratetype_isolate
> > spin_lock_irqsave(zone->lock)
> >
> > free_hot_cold_page(Page A)
> > /* without zone->lock */
> > migratetype =
> > get_pageblock_migratetype(Page A);
> > /*
> > * Page could be moved into MIGRATE_MOVABLE
> > * of per_cpu_pages
> > */
> > list_add_tail(>lru,
> > >lists[migratetype]);
> >
> > set_pageblock_isolate
> > move_freepages_block
> > drain_all_pages
> >
> > /* Page A could be in MIGRATE_MOVABLE of
> > free_list. */
> >
> > check_pages_isolated
> > __test_page_isolated_in_pageblock
> > /*
> > * We can't catch freed page which
> > * is free_list[MIGRATE_MOVABLE]
> > */
> > if (PageBuddy(page A))
> > pfn += 1 << page_order(page A);
> >
> > /* So, Page A could be allocated */
> >
> > __offline_isolated_pages
> > /*
> > * BUG_ON hit or offline page
> > * which is used by someone
> > */
> > BUG_ON(!PageBuddy(page A));
> >
> > Signed-off-by: Minchan Kim
> > ---
> > I found this problem during code review so please confirm it.
> > Kame?
> >
> > mm/page_isolation.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> > index acf65a7..4699d1f 100644
> > --- a/mm/page_isolation.c
> > +++ b/mm/page_isolation.c
> > @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
> > unsigned long end_pfn)
> > continue;
> > }
> > page = pfn_to_page(pfn);
> > - if (PageBuddy(page))
> > + if (PageBuddy(page)) {
> > pfn += 1 << page_order(page);
> > + if (get_page_migratetype(page) != MIGRATE_ISOLATE)
> > + break;
> > + }
>
> test_page_isolated() already have check
> get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
>
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the page
would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that race
happens.
--
Kind regards,
Minchan Kim
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim wrote:
> Like below, memory-hotplug makes race between page-isolation
> and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
>
> CPU A CPU B
>
> start_isolate_page_range
> set_migratetype_isolate
> spin_lock_irqsave(zone->lock)
>
> free_hot_cold_page(Page A)
> /* without zone->lock */
> migratetype = get_pageblock_migratetype(Page
> A);
> /*
> * Page could be moved into MIGRATE_MOVABLE
> * of per_cpu_pages
> */
> list_add_tail(>lru,
> >lists[migratetype]);
>
> set_pageblock_isolate
> move_freepages_block
> drain_all_pages
>
> /* Page A could be in MIGRATE_MOVABLE of
> free_list. */
>
> check_pages_isolated
> __test_page_isolated_in_pageblock
> /*
> * We can't catch freed page which
> * is free_list[MIGRATE_MOVABLE]
> */
> if (PageBuddy(page A))
> pfn += 1 << page_order(page A);
>
> /* So, Page A could be allocated */
>
> __offline_isolated_pages
> /*
> * BUG_ON hit or offline page
> * which is used by someone
> */
> BUG_ON(!PageBuddy(page A));
>
> Signed-off-by: Minchan Kim
> ---
> I found this problem during code review so please confirm it.
> Kame?
>
> mm/page_isolation.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index acf65a7..4699d1f 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
> unsigned long end_pfn)
> continue;
> }
> page = pfn_to_page(pfn);
> - if (PageBuddy(page))
> + if (PageBuddy(page)) {
> pfn += 1 << page_order(page);
> + if (get_page_migratetype(page) != MIGRATE_ISOLATE)
> + break;
> + }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
> else if (page_count(page) == 0 &&
> get_page_migratetype(page) == MIGRATE_ISOLATE)
> pfn += 1;
> --
> 1.7.9.5
>
--
Regards,
--Bob
--
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/
[RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone->lock)
free_hot_cold_page(Page A)
/* without zone->lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(>lru,
>lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 << page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 << page_order(page);
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
+ }
else if (page_count(page) == 0 &&
get_page_migratetype(page) == MIGRATE_ISOLATE)
pfn += 1;
--
1.7.9.5
--
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/
[RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype = get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
+ }
else if (page_count(page) == 0
get_page_migratetype(page) == MIGRATE_ISOLATE)
pfn += 1;
--
1.7.9.5
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype = get_pageblock_migratetype(Page
A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
else if (page_count(page) == 0
get_page_migratetype(page) == MIGRATE_ISOLATE)
pfn += 1;
--
1.7.9.5
--
Regards,
--Bob
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the page
would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that race
happens.
--
Kind regards,
Minchan Kim
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn,
unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) != MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
--
Thanks,
--Bob
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into
MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE of
free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
pfn, unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) !=
MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on pcp, not
free_list.
--
Thanks,
--Bob
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
Kind regards,
Minchan Kim
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into
MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
pfn, unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) !=
MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but the
page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if that
race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on pcp,
not free_list.
Got it, then why not just drain pcp pages before move_freepages_block() ?
And I didn't see the effect by adding the check if
(get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
Since set_pageblock_isolate() have been called by CPU A, this check
will always false which cause CPU A still consider Page A isolated,
then PAGE A still can be allocated by CPU B from pcp.
--
Regards,
--Bob
--
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/
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into
MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
pfn, unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) !=
MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but
the page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
that race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on pcp,
not free_list.
Got it, then why not just drain pcp pages before move_freepages_block() ?
CPU A CPU B
drain_all_pages
lock(zone-lock);
free_hot_cold_page
MIGRATE_MOVABLE =
get_pageblock_migratetype(page);
list_add(page-lru, pcp-lists[migratetype])
set_pageblock_isolate
move_free_pages_block
unlock(zone-lock);
We can't make it atomic.
And I didn't see the effect by adding the check if
(get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
Since set_pageblock_isolate() have been called by CPU A, this check
will always false which cause CPU A still consider Page A isolated,
then PAGE A still can be allocated by CPU B from pcp.
Please don't confuse get_page_migratetype and get_pageblock_migratetype.
get_page_migratetype returns migratetype inforamtion of *page* which is
in free_list while get_pageblock_migratetype returns *pageblock*'s migratetype.
--
Regards,
--Bob
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list:
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 11:56 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org
wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in __offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into
MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in
MIGRATE_MOVABLE of free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned long
pfn, unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) !=
MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE but
the page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
that race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on pcp,
not free_list.
Got it, then why not just drain pcp pages before move_freepages_block() ?
CPU A CPU B
drain_all_pages
lock(zone-lock);
free_hot_cold_page
MIGRATE_MOVABLE =
get_pageblock_migratetype(page);
list_add(page-lru, pcp-lists[migratetype])
set_pageblock_isolate
move_free_pages_block
unlock(zone-lock);
We can't make it atomic.
And I didn't see the effect by adding the check if
(get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
Since set_pageblock_isolate() have been called by CPU A, this check
will always false which cause CPU A still consider Page A isolated,
then PAGE A still can be allocated by CPU B from pcp.
Please don't confuse get_page_migratetype and get_pageblock_migratetype.
get_page_migratetype returns migratetype inforamtion of *page* which is
in free_list while get_pageblock_migratetype returns *pageblock*'s
migratetype.
Oh, yes i mixed them up.
Last question, it's better to break only before add pfn.
like:
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
+ if (get_page_migratetype(page) !=
Re: [RFC 3/3] memory-hotplug: bug fix race between isolation and allocation
On Wed, Jul 18, 2012 at 01:29:47PM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 11:56 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 11:12:27AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 10:41 AM, Minchan Kim minc...@kernel.org wrote:
On Wed, Jul 18, 2012 at 10:12:57AM +0800, Bob Liu wrote:
On Wed, Jul 18, 2012 at 7:40 AM, Minchan Kim minc...@kernel.org wrote:
Hi Bob,
On Tue, Jul 17, 2012 at 06:13:17PM +0800, Bob Liu wrote:
Hi Minchan,
On Tue, Jul 17, 2012 at 3:01 PM, Minchan Kim minc...@kernel.org
wrote:
Like below, memory-hotplug makes race between page-isolation
and page-allocation so it can hit BUG_ON in
__offline_isolated_pages.
CPU A CPU B
start_isolate_page_range
set_migratetype_isolate
spin_lock_irqsave(zone-lock)
free_hot_cold_page(Page A)
/* without zone-lock */
migratetype =
get_pageblock_migratetype(Page A);
/*
* Page could be moved into
MIGRATE_MOVABLE
* of per_cpu_pages
*/
list_add_tail(page-lru,
pcp-lists[migratetype]);
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in
MIGRATE_MOVABLE of free_list. */
check_pages_isolated
__test_page_isolated_in_pageblock
/*
* We can't catch freed page which
* is free_list[MIGRATE_MOVABLE]
*/
if (PageBuddy(page A))
pfn += 1 page_order(page A);
/* So, Page A could be allocated */
__offline_isolated_pages
/*
* BUG_ON hit or offline page
* which is used by someone
*/
BUG_ON(!PageBuddy(page A));
Signed-off-by: Minchan Kim minc...@kernel.org
---
I found this problem during code review so please confirm it.
Kame?
mm/page_isolation.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index acf65a7..4699d1f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -196,8 +196,11 @@ __test_page_isolated_in_pageblock(unsigned
long pfn, unsigned long end_pfn)
continue;
}
page = pfn_to_page(pfn);
- if (PageBuddy(page))
+ if (PageBuddy(page)) {
pfn += 1 page_order(page);
+ if (get_page_migratetype(page) !=
MIGRATE_ISOLATE)
+ break;
+ }
test_page_isolated() already have check
get_pageblock_migratetype(page) != MIGRATE_ISOLATE.
That's why I send a patch.
As I describe in description, pageblock migration type of
get_page_migratetype(page)
is inconsistent with free_list[migrationtype].
I mean get_pageblock_migratetype(page) will return MIGRATE_ISOLATE
but the page would be
in free_list[MIGRATE_MOVABLE] so it could be allocated for someone if
that race happens.
Sorry, I'm still not get the situation how this race happens.
set_pageblock_isolate
move_freepages_block
drain_all_pages
/* Page A could be in MIGRATE_MOVABLE
of free_list. */
I think move_freepages_block() will call list_move() to move Page A to
free_list[MIGRATE_ISOLATE], so this case can't happen?
move_freepages_block handles only pages in free_list but Page A is on
pcp, not free_list.
Got it, then why not just drain pcp pages before move_freepages_block() ?
CPU A CPU B
drain_all_pages
lock(zone-lock);
free_hot_cold_page
MIGRATE_MOVABLE =
get_pageblock_migratetype(page);
list_add(page-lru,
pcp-lists[migratetype])
set_pageblock_isolate
move_free_pages_block
unlock(zone-lock);
We can't make it atomic.
And I didn't see the effect by adding the check if
(get_page_migratetype(page) != MIGRATE_ISOLATE) for this race.
Since set_pageblock_isolate() have been called by CPU A, this check
will always false which cause CPU A still consider Page A isolated,
then PAGE A still can be allocated by CPU B from pcp.
Please don't confuse get_page_migratetype and get_pageblock_migratetype.
get_page_migratetype returns migratetype inforamtion of *page* which is
in free_list while get_pageblock_migratetype returns *pageblock*'s
migratetype.
Oh, yes i mixed
