[PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[Pankaj Gupta]

While debugging issue in realtime kernel i found a scenario
which resulted in infinite loop resulting because of empty pcp->lists
and valid 'to_free' value. This patch is to add 'WARN_ON' in function
'free_pcppages_bulk' if there is possibility of infinite loop because 
of any bug in code.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
 mm/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3fbe73a..07d3080 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1096,6 +1096,8 @@ static void free_pcppages_bulk(struct zone *zone, int count,
 			if (++migratetype == MIGRATE_PCPTYPES)
 				migratetype = 0;
 			list = &pcp->lists[migratetype];
+
+			WARN_ON(batch_free > MIGRATE_PCPTYPES);
 		} while (list_empty(list));
 
 		/* This is the only non-empty list. Free them all. */
-- 
2.7.4

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

Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[Mel Gorman]

On Fri, Aug 19, 2016 at 05:45:18PM +0530, Pankaj Gupta wrote:
> While debugging issue in realtime kernel i found a scenario
> which resulted in infinite loop resulting because of empty pcp->lists
> and valid 'to_free' value. This patch is to add 'WARN_ON' in function
> 'free_pcppages_bulk' if there is possibility of infinite loop because 
> of any bug in code.
> 

What was the bug that allowed this situation to occur? It would imply
the pcp count was somehow out of sync.

-- 
Mel Gorman
SUSE Labs

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

Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[Pankaj Gupta]

> 
> On Fri, Aug 19, 2016 at 05:45:18PM +0530, Pankaj Gupta wrote:
> > While debugging issue in realtime kernel i found a scenario
> > which resulted in infinite loop resulting because of empty pcp->lists
> > and valid 'to_free' value. This patch is to add 'WARN_ON' in function
> > 'free_pcppages_bulk' if there is possibility of infinite loop because
> > of any bug in code.
> > 
> 
> What was the bug that allowed this situation to occur? It would imply
> the pcp count was somehow out of sync.

Yes pcp count was out of sync. It was a bug in the downstream code.
> 
> --
> Mel Gorman
> SUSE Labs
> 

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

Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[Mel Gorman]

On Fri, Aug 19, 2016 at 09:07:21AM -0400, Pankaj Gupta wrote:
> 
> > 
> > On Fri, Aug 19, 2016 at 05:45:18PM +0530, Pankaj Gupta wrote:
> > > While debugging issue in realtime kernel i found a scenario
> > > which resulted in infinite loop resulting because of empty pcp->lists
> > > and valid 'to_free' value. This patch is to add 'WARN_ON' in function
> > > 'free_pcppages_bulk' if there is possibility of infinite loop because
> > > of any bug in code.
> > > 
> > 
> > What was the bug that allowed this situation to occur? It would imply
> > the pcp count was somehow out of sync.
> 
> Yes pcp count was out of sync. It was a bug in the downstream code.

If the bug is not in the mainline code, I think it would be inappropriate
to add unnecessary code to a relatively hot path. At most, it should be
a VM_BUG_ON but the soft lockup should be clear enough.

-- 
Mel Gorman
SUSE Labs

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

Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk'

[Pankaj Gupta]

> > > > While debugging issue in realtime kernel i found a scenario
> > > > which resulted in infinite loop resulting because of empty pcp->lists
> > > > and valid 'to_free' value. This patch is to add 'WARN_ON' in function
> > > > 'free_pcppages_bulk' if there is possibility of infinite loop because
> > > > of any bug in code.
> > > > 
> > > 
> > > What was the bug that allowed this situation to occur? It would imply
> > > the pcp count was somehow out of sync.
> > 
> > Yes pcp count was out of sync. It was a bug in the downstream code.
> 
> If the bug is not in the mainline code, I think it would be inappropriate
> to add unnecessary code to a relatively hot path. At most, it should be
> a VM_BUG_ON but the soft lockup should be clear enough.

yes 'VM_BUG_ON' is right thing here. This could help in realtime kernel where
'free_pcppages_bulk' is divided into two functions 'isolate_pcp_pages' and 
'free_pcppages_bulk' where 'isolate_pcp_pages' isolate the 'batch/count' number
of pages and 'free_pcppages_bulk' just free these pages.

I was just thinking if there is any possibility of out of sync with count and temporary
list this might help. But looking more at the code does not seems like there is any 
chance until any other potential bug somewhere else in code result this scenario.

I will drop this patch.

Thanks for the review.

> 
> --
> Mel Gorman
> SUSE Labs
> 

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