Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1466327 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages_bulk' Pankaj Gupta <pagupta@redhat.com> - 2016-08-19 14:20 +0200
    Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if  empty lists in free_pcppages_bulk' Mel Gorman <mgorman@techsingularity.net> - 2016-08-19 14:50 +0200
      Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if  empty lists in free_pcppages_bulk' Pankaj Gupta <pagupta@redhat.com> - 2016-08-19 15:10 +0200
        Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if  empty lists in free_pcppages_bulk' Mel Gorman <mgorman@techsingularity.net> - 2016-08-19 16:10 +0200
          Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if  empty lists in free_pcppages_bulk' Pankaj Gupta <pagupta@redhat.com> - 2016-08-23 10:50 +0200

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

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web