Re: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
On Thu 12-07-12 18:05:19, Minchan Kim wrote: > Hi Michal, > > On Thu, Jul 12, 2012 at 10:19:23AM +0200, Michal Hocko wrote: > > On Thu 12-07-12 11:50:48, Minchan Kim wrote: > > > In __zone_watermark_ok, free and min are signed long type > > > while z->lowmem_reserve[classzone_idx] is unsigned long type. > > > So comparision of them could be wrong due to type conversion > > > to unsigned although free_pages is minus value. > > > > Agreed on that > > but > > > > > > It could return true instead of false in case of order-0 check > > > so that kswapd could sleep forever. > > > > I am kind of lost here. How can we have negative free_pages with > > order-0? It would need to come with a negative value because > > free_pages -= (1 << order) - 1; > > won't make it negative. > > Right you are. I missed that part. > > The reason Aaditya reported this problem is caused by my patch[1] in > this series. zone_watermark_ok_safe didn't reset free_pages to zero > although free_pages becomes minus value(Please, look at [1]) > > [1] memory-hotplug: fix kswapd looping forever problem > > I think we have no problem in current code because if order isn't zero > and free_pages is minus value, it could exit in the middle of loop with > false. Yes, but it would be still nice to not rely on the loop and make the first test effective. So I think the patch still makes sense. > > So I think it was totally patch's problem. :( > But I agree auto type casting problem is subtle error-prone so it's valuable > to fix, too. As you mentiond, I should rewrite description and subject for it. > > Thanks for the review! > > > > > > It means livelock because direct reclaimer loops forever until kswapd > > > set zone->all_unreclaimable. > > > > > > Aaditya reported this problem when he test my hotplug patch. > > > > > > Reported-off-by: Aaditya Kumar > > > Tested-by: Aaditya Kumar > > > Signed-off-by: Aaditya Kumar > > > Signed-off-by: Minchan Kim > > > > So you can add my Reviewed-by: Michal Hocko > > but the changelog could be more clear. > > > > > --- > > > This patch isn't dependent with this series. > > > It seems to be candidate for -stable but I'm not sure because of this > > > part. > > > So, pass the decision to akpm. > > > > > > " - It must fix a real bug that bothers people (not a, "This could be a > > >problem..." type thing)." > > > > I am wondering what Testted-by means if "This could be a problem..." > > type thing)." > > He reported it during testing this series so I don't think it will happen > in current code because zone_page_state and zone_page_state_snapshot > can't set free_pages to minus. > > > > > > > > > mm/page_alloc.c |3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > > index f17e6e4..627653c 100644 > > > --- a/mm/page_alloc.c > > > +++ b/mm/page_alloc.c > > > @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int > > > order, unsigned long mark, > > > { > > > /* free_pages my go negative - that's OK */ > > > long min = mark; > > > + long lowmem_reserve = z->lowmem_reserve[classzone_idx]; > > > int o; > > > > > > free_pages -= (1 << order) - 1; > > > @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int > > > order, unsigned long mark, > > > if (alloc_flags & ALLOC_HARDER) > > > min -= min / 4; > > > > > > - if (free_pages <= min + z->lowmem_reserve[classzone_idx]) > > > + if (free_pages <= min + lowmem_reserve) > > > return false; > > > for (o = 0; o < order; o++) { > > > /* At the next order, this order's pages become unavailable */ > > > -- > > > 1.7.9.5 > > > > > > -- > > > 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 > > > > -- > > Michal Hocko > > SUSE Labs > > SUSE LINUX s.r.o. > > Lihovarska 1060/12 > > 190 00 Praha 9 > > Czech Republic > > > > -- > > 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
Hi Michal, On Thu, Jul 12, 2012 at 10:19:23AM +0200, Michal Hocko wrote: > On Thu 12-07-12 11:50:48, Minchan Kim wrote: > > In __zone_watermark_ok, free and min are signed long type > > while z->lowmem_reserve[classzone_idx] is unsigned long type. > > So comparision of them could be wrong due to type conversion > > to unsigned although free_pages is minus value. > > Agreed on that > but > > > > It could return true instead of false in case of order-0 check > > so that kswapd could sleep forever. > > I am kind of lost here. How can we have negative free_pages with > order-0? It would need to come with a negative value because > free_pages -= (1 << order) - 1; > won't make it negative. Right you are. I missed that part. The reason Aaditya reported this problem is caused by my patch[1] in this series. zone_watermark_ok_safe didn't reset free_pages to zero although free_pages becomes minus value(Please, look at [1]) [1] memory-hotplug: fix kswapd looping forever problem I think we have no problem in current code because if order isn't zero and free_pages is minus value, it could exit in the middle of loop with false. So I think it was totally patch's problem. :( But I agree auto type casting problem is subtle error-prone so it's valuable to fix, too. As you mentiond, I should rewrite description and subject for it. Thanks for the review! > > > It means livelock because direct reclaimer loops forever until kswapd > > set zone->all_unreclaimable. > > > > Aaditya reported this problem when he test my hotplug patch. > > > > Reported-off-by: Aaditya Kumar > > Tested-by: Aaditya Kumar > > Signed-off-by: Aaditya Kumar > > Signed-off-by: Minchan Kim > > So you can add my Reviewed-by: Michal Hocko > but the changelog could be more clear. > > > --- > > This patch isn't dependent with this series. > > It seems to be candidate for -stable but I'm not sure because of this part. > > So, pass the decision to akpm. > > > > " - It must fix a real bug that bothers people (not a, "This could be a > >problem..." type thing)." > > I am wondering what Testted-by means if "This could be a problem..." > type thing)." He reported it during testing this series so I don't think it will happen in current code because zone_page_state and zone_page_state_snapshot can't set free_pages to minus. > > > > > mm/page_alloc.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index f17e6e4..627653c 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int > > order, unsigned long mark, > > { > > /* free_pages my go negative - that's OK */ > > long min = mark; > > + long lowmem_reserve = z->lowmem_reserve[classzone_idx]; > > int o; > > > > free_pages -= (1 << order) - 1; > > @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int > > order, unsigned long mark, > > if (alloc_flags & ALLOC_HARDER) > > min -= min / 4; > > > > - if (free_pages <= min + z->lowmem_reserve[classzone_idx]) > > + if (free_pages <= min + lowmem_reserve) > > return false; > > for (o = 0; o < order; o++) { > > /* At the next order, this order's pages become unavailable */ > > -- > > 1.7.9.5 > > > > -- > > 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 > > -- > Michal Hocko > SUSE Labs > SUSE LINUX s.r.o. > Lihovarska 1060/12 > 190 00 Praha 9 > Czech Republic > > -- > 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: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
On Thu 12-07-12 11:50:48, Minchan Kim wrote: > In __zone_watermark_ok, free and min are signed long type > while z->lowmem_reserve[classzone_idx] is unsigned long type. > So comparision of them could be wrong due to type conversion > to unsigned although free_pages is minus value. Agreed on that but > > It could return true instead of false in case of order-0 check > so that kswapd could sleep forever. I am kind of lost here. How can we have negative free_pages with order-0? It would need to come with a negative value because free_pages -= (1 << order) - 1; won't make it negative. > It means livelock because direct reclaimer loops forever until kswapd > set zone->all_unreclaimable. > > Aaditya reported this problem when he test my hotplug patch. > > Reported-off-by: Aaditya Kumar > Tested-by: Aaditya Kumar > Signed-off-by: Aaditya Kumar > Signed-off-by: Minchan Kim So you can add my Reviewed-by: Michal Hocko but the changelog could be more clear. > --- > This patch isn't dependent with this series. > It seems to be candidate for -stable but I'm not sure because of this part. > So, pass the decision to akpm. > > " - It must fix a real bug that bothers people (not a, "This could be a >problem..." type thing)." I am wondering what Testted-by means if "This could be a problem..." type thing)." > > mm/page_alloc.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index f17e6e4..627653c 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int > order, unsigned long mark, > { > /* free_pages my go negative - that's OK */ > long min = mark; > + long lowmem_reserve = z->lowmem_reserve[classzone_idx]; > int o; > > free_pages -= (1 << order) - 1; > @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int > order, unsigned long mark, > if (alloc_flags & ALLOC_HARDER) > min -= min / 4; > > - if (free_pages <= min + z->lowmem_reserve[classzone_idx]) > + if (free_pages <= min + lowmem_reserve) > return false; > for (o = 0; o < order; o++) { > /* At the next order, this order's pages become unavailable */ > -- > 1.7.9.5 > > -- > 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
On Thu 12-07-12 11:50:48, Minchan Kim wrote: In __zone_watermark_ok, free and min are signed long type while z-lowmem_reserve[classzone_idx] is unsigned long type. So comparision of them could be wrong due to type conversion to unsigned although free_pages is minus value. Agreed on that but It could return true instead of false in case of order-0 check so that kswapd could sleep forever. I am kind of lost here. How can we have negative free_pages with order-0? It would need to come with a negative value because free_pages -= (1 order) - 1; won't make it negative. It means livelock because direct reclaimer loops forever until kswapd set zone-all_unreclaimable. Aaditya reported this problem when he test my hotplug patch. Reported-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Tested-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Minchan Kim minc...@kernel.org So you can add my Reviewed-by: Michal Hocko mho...@suse.cz but the changelog could be more clear. --- This patch isn't dependent with this series. It seems to be candidate for -stable but I'm not sure because of this part. So, pass the decision to akpm. - It must fix a real bug that bothers people (not a, This could be a problem... type thing). I am wondering what Testted-by means if This could be a problem... type thing). mm/page_alloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f17e6e4..627653c 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, { /* free_pages my go negative - that's OK */ long min = mark; + long lowmem_reserve = z-lowmem_reserve[classzone_idx]; int o; free_pages -= (1 order) - 1; @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, if (alloc_flags ALLOC_HARDER) min -= min / 4; - if (free_pages = min + z-lowmem_reserve[classzone_idx]) + if (free_pages = min + lowmem_reserve) return false; for (o = 0; o order; o++) { /* At the next order, this order's pages become unavailable */ -- 1.7.9.5 -- 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
Hi Michal, On Thu, Jul 12, 2012 at 10:19:23AM +0200, Michal Hocko wrote: On Thu 12-07-12 11:50:48, Minchan Kim wrote: In __zone_watermark_ok, free and min are signed long type while z-lowmem_reserve[classzone_idx] is unsigned long type. So comparision of them could be wrong due to type conversion to unsigned although free_pages is minus value. Agreed on that but It could return true instead of false in case of order-0 check so that kswapd could sleep forever. I am kind of lost here. How can we have negative free_pages with order-0? It would need to come with a negative value because free_pages -= (1 order) - 1; won't make it negative. Right you are. I missed that part. The reason Aaditya reported this problem is caused by my patch[1] in this series. zone_watermark_ok_safe didn't reset free_pages to zero although free_pages becomes minus value(Please, look at [1]) [1] memory-hotplug: fix kswapd looping forever problem I think we have no problem in current code because if order isn't zero and free_pages is minus value, it could exit in the middle of loop with false. So I think it was totally patch's problem. :( But I agree auto type casting problem is subtle error-prone so it's valuable to fix, too. As you mentiond, I should rewrite description and subject for it. Thanks for the review! It means livelock because direct reclaimer loops forever until kswapd set zone-all_unreclaimable. Aaditya reported this problem when he test my hotplug patch. Reported-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Tested-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Minchan Kim minc...@kernel.org So you can add my Reviewed-by: Michal Hocko mho...@suse.cz but the changelog could be more clear. --- This patch isn't dependent with this series. It seems to be candidate for -stable but I'm not sure because of this part. So, pass the decision to akpm. - It must fix a real bug that bothers people (not a, This could be a problem... type thing). I am wondering what Testted-by means if This could be a problem... type thing). He reported it during testing this series so I don't think it will happen in current code because zone_page_state and zone_page_state_snapshot can't set free_pages to minus. mm/page_alloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f17e6e4..627653c 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, { /* free_pages my go negative - that's OK */ long min = mark; + long lowmem_reserve = z-lowmem_reserve[classzone_idx]; int o; free_pages -= (1 order) - 1; @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, if (alloc_flags ALLOC_HARDER) min -= min / 4; - if (free_pages = min + z-lowmem_reserve[classzone_idx]) + if (free_pages = min + lowmem_reserve) return false; for (o = 0; o order; o++) { /* At the next order, this order's pages become unavailable */ -- 1.7.9.5 -- 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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: [PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
On Thu 12-07-12 18:05:19, Minchan Kim wrote: Hi Michal, On Thu, Jul 12, 2012 at 10:19:23AM +0200, Michal Hocko wrote: On Thu 12-07-12 11:50:48, Minchan Kim wrote: In __zone_watermark_ok, free and min are signed long type while z-lowmem_reserve[classzone_idx] is unsigned long type. So comparision of them could be wrong due to type conversion to unsigned although free_pages is minus value. Agreed on that but It could return true instead of false in case of order-0 check so that kswapd could sleep forever. I am kind of lost here. How can we have negative free_pages with order-0? It would need to come with a negative value because free_pages -= (1 order) - 1; won't make it negative. Right you are. I missed that part. The reason Aaditya reported this problem is caused by my patch[1] in this series. zone_watermark_ok_safe didn't reset free_pages to zero although free_pages becomes minus value(Please, look at [1]) [1] memory-hotplug: fix kswapd looping forever problem I think we have no problem in current code because if order isn't zero and free_pages is minus value, it could exit in the middle of loop with false. Yes, but it would be still nice to not rely on the loop and make the first test effective. So I think the patch still makes sense. So I think it was totally patch's problem. :( But I agree auto type casting problem is subtle error-prone so it's valuable to fix, too. As you mentiond, I should rewrite description and subject for it. Thanks for the review! It means livelock because direct reclaimer loops forever until kswapd set zone-all_unreclaimable. Aaditya reported this problem when he test my hotplug patch. Reported-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Tested-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Minchan Kim minc...@kernel.org So you can add my Reviewed-by: Michal Hocko mho...@suse.cz but the changelog could be more clear. --- This patch isn't dependent with this series. It seems to be candidate for -stable but I'm not sure because of this part. So, pass the decision to akpm. - It must fix a real bug that bothers people (not a, This could be a problem... type thing). I am wondering what Testted-by means if This could be a problem... type thing). He reported it during testing this series so I don't think it will happen in current code because zone_page_state and zone_page_state_snapshot can't set free_pages to minus. mm/page_alloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f17e6e4..627653c 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, { /* free_pages my go negative - that's OK */ long min = mark; + long lowmem_reserve = z-lowmem_reserve[classzone_idx]; int o; free_pages -= (1 order) - 1; @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, if (alloc_flags ALLOC_HARDER) min -= min / 4; - if (free_pages = min + z-lowmem_reserve[classzone_idx]) + if (free_pages = min + lowmem_reserve) return false; for (o = 0; o order; o++) { /* At the next order, this order's pages become unavailable */ -- 1.7.9.5 -- 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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/
[PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
In __zone_watermark_ok, free and min are signed long type while z->lowmem_reserve[classzone_idx] is unsigned long type. So comparision of them could be wrong due to type conversion to unsigned although free_pages is minus value. It could return true instead of false in case of order-0 check so that kswapd could sleep forever. It means livelock because direct reclaimer loops forever until kswapd set zone->all_unreclaimable. Aaditya reported this problem when he test my hotplug patch. Reported-off-by: Aaditya Kumar Tested-by: Aaditya Kumar Signed-off-by: Aaditya Kumar Signed-off-by: Minchan Kim --- This patch isn't dependent with this series. It seems to be candidate for -stable but I'm not sure because of this part. So, pass the decision to akpm. " - It must fix a real bug that bothers people (not a, "This could be a problem..." type thing)." mm/page_alloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f17e6e4..627653c 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, { /* free_pages my go negative - that's OK */ long min = mark; + long lowmem_reserve = z->lowmem_reserve[classzone_idx]; int o; free_pages -= (1 << order) - 1; @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, if (alloc_flags & ALLOC_HARDER) min -= min / 4; - if (free_pages <= min + z->lowmem_reserve[classzone_idx]) + if (free_pages <= min + lowmem_reserve) return false; for (o = 0; o < order; o++) { /* At the next order, this order's pages become unavailable */ -- 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/
[PATCH 2/3 v3] mm: bug fix free page check in zone_watermark_ok
In __zone_watermark_ok, free and min are signed long type while z-lowmem_reserve[classzone_idx] is unsigned long type. So comparision of them could be wrong due to type conversion to unsigned although free_pages is minus value. It could return true instead of false in case of order-0 check so that kswapd could sleep forever. It means livelock because direct reclaimer loops forever until kswapd set zone-all_unreclaimable. Aaditya reported this problem when he test my hotplug patch. Reported-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Tested-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Aaditya Kumar aaditya.ku...@ap.sony.com Signed-off-by: Minchan Kim minc...@kernel.org --- This patch isn't dependent with this series. It seems to be candidate for -stable but I'm not sure because of this part. So, pass the decision to akpm. - It must fix a real bug that bothers people (not a, This could be a problem... type thing). mm/page_alloc.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index f17e6e4..627653c 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1594,6 +1594,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, { /* free_pages my go negative - that's OK */ long min = mark; + long lowmem_reserve = z-lowmem_reserve[classzone_idx]; int o; free_pages -= (1 order) - 1; @@ -1602,7 +1603,7 @@ static bool __zone_watermark_ok(struct zone *z, int order, unsigned long mark, if (alloc_flags ALLOC_HARDER) min -= min / 4; - if (free_pages = min + z-lowmem_reserve[classzone_idx]) + if (free_pages = min + lowmem_reserve) return false; for (o = 0; o order; o++) { /* At the next order, this order's pages become unavailable */ -- 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/