[PATCH] rcu: restore correct batch limiting

[PATCH] rcu: restore correct batch limiting

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
invocation) added a regression in rcu_do_batch()

Under stress, RCU is supposed to allow to process all items in queue,
instead of a batch of 10 items (blimit), but an integer overflow makes
the effective limit being 1.

So RCU cannot recover and machine eventually crash because of OOM.

Using long instead of int is not really needed, convert everything
to integers.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Google-Bug-Id: 7362491
CC: stable@vger.kernel.org
---
 kernel/rcutree.c       |    4 ++--
 kernel/rcutree.h       |    4 ++--
 kernel/rcutree_trace.c |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 74df86b..fc3ebae 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1834,7 +1834,7 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
 	rdp->n_cbs_invoked += count;
 
 	/* Reinstate batch limit if we have worked down the excess. */
-	if (rdp->blimit == LONG_MAX && rdp->qlen <= qlowmark)
+	if (rdp->blimit == INT_MAX && rdp->qlen <= qlowmark)
 		rdp->blimit = blimit;
 
 	/* Reset ->qlen_last_fqs_check trigger if enough CBs have drained. */
@@ -2097,7 +2097,7 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,
 			rcu_start_gp(rsp, nestflag);  /* rlses rnp_root->lock */
 		} else {
 			/* Give the grace period a kick. */
-			rdp->blimit = LONG_MAX;
+			rdp->blimit = INT_MAX;
 			if (rsp->n_force_qs == rdp->n_force_qs_snap &&
 			    *rdp->nxttail[RCU_DONE_TAIL] != head)
 				force_quiescent_state(rsp);
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index a240f03..6421173 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -291,11 +291,11 @@ struct rcu_data {
 	unsigned long   n_cbs_adopted;  /* RCU cbs adopted from dying CPU */
 	unsigned long	n_force_qs_snap;
 					/* did other CPU force QS recently? */
-	long		blimit;		/* Upper limit on a processed batch */
+	int		blimit;		/* Upper limit on a processed batch */
 
 	/* 3) dynticks interface. */
+	int		dynticks_snap;	/* Per-GP tracking for dynticks. */
 	struct rcu_dynticks *dynticks;	/* Shared per-CPU dynticks state. */
-	int dynticks_snap;		/* Per-GP tracking for dynticks. */
 
 	/* 4) reasons this CPU needed to be kicked by force_quiescent_state */
 	unsigned long dynticks_fqs;	/* Kicked due to dynticks idle. */
diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
index 693513b..fde0541 100644
--- a/kernel/rcutree_trace.c
+++ b/kernel/rcutree_trace.c
@@ -113,7 +113,7 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp)
 					  rdp->cpu)),
 		   per_cpu(rcu_cpu_kthread_loops, rdp->cpu) & 0xffff);
 #endif /* #ifdef CONFIG_RCU_BOOST */
-	seq_printf(m, " b=%ld", rdp->blimit);
+	seq_printf(m, " b=%d", rdp->blimit);
 	seq_printf(m, " ci=%lu co=%lu ca=%lu\n",
 		   rdp->n_cbs_invoked, rdp->n_cbs_orphaned, rdp->n_cbs_adopted);
 }
@@ -173,7 +173,7 @@ static void print_one_rcu_data_csv(struct seq_file *m, struct rcu_data *rdp)
 		   convert_kthread_status(per_cpu(rcu_cpu_kthread_status,
 					  rdp->cpu)));
 #endif /* #ifdef CONFIG_RCU_BOOST */
-	seq_printf(m, ",%ld", rdp->blimit);
+	seq_printf(m, ",%d", rdp->blimit);
 	seq_printf(m, ",%lu,%lu,%lu\n",
 		   rdp->n_cbs_invoked, rdp->n_cbs_orphaned, rdp->n_cbs_adopted);
 }

Re: [PATCH] rcu: restore correct batch limiting

[Hillf Danton]

Hi Eric,

On Wed, Oct 17, 2012 at 3:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> invocation) added a regression in rcu_do_batch()
>
> Under stress, RCU is supposed to allow to process all items in queue,
> instead of a batch of 10 items (blimit), but an integer overflow makes
> the effective limit being 1.
>
> So RCU cannot recover and machine eventually crash because of OOM.
>
> Using long instead of int is not really needed, convert everything
> to integers.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Google-Bug-Id: 7362491

That means over 7 million bugs accumulated?

> CC: stable@vger.kernel.org
> ---
>  kernel/rcutree.c       |    4 ++--
>  kernel/rcutree.h       |    4 ++--
>  kernel/rcutree_trace.c |    4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index 74df86b..fc3ebae 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1834,7 +1834,7 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
>         rdp->n_cbs_invoked += count;
>
>         /* Reinstate batch limit if we have worked down the excess. */
> -       if (rdp->blimit == LONG_MAX && rdp->qlen <= qlowmark)
> +       if (rdp->blimit == INT_MAX && rdp->qlen <= qlowmark)
>                 rdp->blimit = blimit;
>
>         /* Reset ->qlen_last_fqs_check trigger if enough CBs have drained. */
> @@ -2097,7 +2097,7 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,
>                         rcu_start_gp(rsp, nestflag);  /* rlses rnp_root->lock */
>                 } else {
>                         /* Give the grace period a kick. */
> -                       rdp->blimit = LONG_MAX;
> +                       rdp->blimit = INT_MAX;
>                         if (rsp->n_force_qs == rdp->n_force_qs_snap &&
>                             *rdp->nxttail[RCU_DONE_TAIL] != head)
>                                 force_quiescent_state(rsp);
> diff --git a/kernel/rcutree.h b/kernel/rcutree.h
> index a240f03..6421173 100644
> --- a/kernel/rcutree.h
> +++ b/kernel/rcutree.h
> @@ -291,11 +291,11 @@ struct rcu_data {
>         unsigned long   n_cbs_adopted;  /* RCU cbs adopted from dying CPU */
>         unsigned long   n_force_qs_snap;
>                                         /* did other CPU force QS recently? */
> -       long            blimit;         /* Upper limit on a processed batch */
> +       int             blimit;         /* Upper limit on a processed batch */
>
>         /* 3) dynticks interface. */
> +       int             dynticks_snap;  /* Per-GP tracking for dynticks. */
>         struct rcu_dynticks *dynticks;  /* Shared per-CPU dynticks state. */
> -       int dynticks_snap;              /* Per-GP tracking for dynticks. */

Hm, no words in change log for shuffling dynticks_snap...

Hillf
>
>         /* 4) reasons this CPU needed to be kicked by force_quiescent_state */
>         unsigned long dynticks_fqs;     /* Kicked due to dynticks idle. */
> diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
> index 693513b..fde0541 100644
> --- a/kernel/rcutree_trace.c
> +++ b/kernel/rcutree_trace.c
> @@ -113,7 +113,7 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp)
>                                           rdp->cpu)),
>                    per_cpu(rcu_cpu_kthread_loops, rdp->cpu) & 0xffff);
>  #endif /* #ifdef CONFIG_RCU_BOOST */
> -       seq_printf(m, " b=%ld", rdp->blimit);
> +       seq_printf(m, " b=%d", rdp->blimit);
>         seq_printf(m, " ci=%lu co=%lu ca=%lu\n",
>                    rdp->n_cbs_invoked, rdp->n_cbs_orphaned, rdp->n_cbs_adopted);
>  }
> @@ -173,7 +173,7 @@ static void print_one_rcu_data_csv(struct seq_file *m, struct rcu_data *rdp)
>                    convert_kthread_status(per_cpu(rcu_cpu_kthread_status,
>                                           rdp->cpu)));
>  #endif /* #ifdef CONFIG_RCU_BOOST */
> -       seq_printf(m, ",%ld", rdp->blimit);
> +       seq_printf(m, ",%d", rdp->blimit);
>         seq_printf(m, ",%lu,%lu,%lu\n",
>                    rdp->n_cbs_invoked, rdp->n_cbs_orphaned, rdp->n_cbs_adopted);
>  }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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] rcu: restore correct batch limiting

[Eric Dumazet]

On Wed, 2012-10-17 at 21:18 +0800, Hillf Danton wrote:

> >                                         /* did other CPU force QS recently? */
> > -       long            blimit;         /* Upper limit on a processed batch */
> > +       int             blimit;         /* Upper limit on a processed batch */
> >
> >         /* 3) dynticks interface. */
> > +       int             dynticks_snap;  /* Per-GP tracking for dynticks. */
> >         struct rcu_dynticks *dynticks;  /* Shared per-CPU dynticks state. */
> > -       int dynticks_snap;              /* Per-GP tracking for dynticks. */
> 
> Hm, no words in change log for shuffling dynticks_snap...

I dont like adding two 32bit holes on a structure ;)

This kind of trivial things dont need a changelog, especially when
fixing a critical bug.

I let Paul add this if he really wants.

Re: [PATCH] rcu: restore correct batch limiting

[Paul E. McKenney]

On Wed, Oct 17, 2012 at 09:14:12AM +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> invocation) added a regression in rcu_do_batch()
> 
> Under stress, RCU is supposed to allow to process all items in queue,
> instead of a batch of 10 items (blimit), but an integer overflow makes
> the effective limit being 1.
> 
> So RCU cannot recover and machine eventually crash because of OOM.
> 
> Using long instead of int is not really needed, convert everything
> to integers.

<facepalm>

Good catch!!!

The reason for favoring long over int is that there are a few systems out
there with terabytes of main memory.  In addition, there have been a few
bugs over the past few years that could result in RCU CPU stalls of more
than a minute.  This makes it impossible to rule out the possibility of
a billion callbacks appearing on one CPU.

So, does the following patch fix things, or a I still confused?

							Thanx, Paul

------------------------------------------------------------------------

rcu: Fix batch-limit size problem

Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
invocation) added a regression in rcu_do_batch()

Under stress, RCU is supposed to allow to process all items in queue,
instead of a batch of 10 items (blimit), but an integer overflow makes
the effective limit being 1.  So, unless there is frequent idle periods
(during which RCU ignores batch limits), RCU can be forced into a
state where it cannot keep up with the callback-generation rate,
eventually resulting in OOM.

This commit therefore converts a few variables in rcu_do_batch() from
int to long to fix this problem.

Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e36d085..e056e1e 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1823,7 +1823,8 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
 {
 	unsigned long flags;
 	struct rcu_head *next, *list, **tail;
-	int bl, count, count_lazy, i;
+	long bl, count, count_lazy;
+	int i;
 
 	/* If no callbacks are ready, just return.*/
 	if (!cpu_has_callbacks_ready_to_invoke(rdp)) {

Re: [PATCH] rcu: restore correct batch limiting

[Eric Dumazet]

On Thu, 2012-10-18 at 04:58 -0700, Paul E. McKenney wrote:
> On Wed, Oct 17, 2012 at 09:14:12AM +0200, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> > invocation) added a regression in rcu_do_batch()
> > 
> > Under stress, RCU is supposed to allow to process all items in queue,
> > instead of a batch of 10 items (blimit), but an integer overflow makes
> > the effective limit being 1.
> > 
> > So RCU cannot recover and machine eventually crash because of OOM.
> > 
> > Using long instead of int is not really needed, convert everything
> > to integers.
> 
> <facepalm>
> 
> Good catch!!!
> 
> The reason for favoring long over int is that there are a few systems out
> there with terabytes of main memory.  In addition, there have been a few
> bugs over the past few years that could result in RCU CPU stalls of more
> than a minute.  This makes it impossible to rule out the possibility of
> a billion callbacks appearing on one CPU.
> 
> So, does the following patch fix things, or a I still confused?
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> rcu: Fix batch-limit size problem
> 
> Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> invocation) added a regression in rcu_do_batch()
> 
> Under stress, RCU is supposed to allow to process all items in queue,
> instead of a batch of 10 items (blimit), but an integer overflow makes
> the effective limit being 1.  So, unless there is frequent idle periods
> (during which RCU ignores batch limits), RCU can be forced into a
> state where it cannot keep up with the callback-generation rate,
> eventually resulting in OOM.
> 
> This commit therefore converts a few variables in rcu_do_batch() from
> int to long to fix this problem.
> 
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index e36d085..e056e1e 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1823,7 +1823,8 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
>  {
>  	unsigned long flags;
>  	struct rcu_head *next, *list, **tail;
> -	int bl, count, count_lazy, i;
> +	long bl, count, count_lazy;
> +	int i;
>  
>  	/* If no callbacks are ready, just return.*/
>  	if (!cpu_has_callbacks_ready_to_invoke(rdp)) {
> 

Yes, why not, but global "int blimit" being an int, I really dont see
the point having a long in struct rcu_data.

Having 2 billions callbacks on one cpu would be problematic, I really
hope nobody relies on this ;)

I guess the 10/infinity switch should be smarter.

something like the following maybe :

rdp->blimit = max(blimit, rdp->qlen >> 6);

(if queue is big, dont wait to hit 10000 before allowing more items to
be handled per round)


Signed-off-by: Eric Dumazet <edumazet@google.com>

Please dont forget stable teams.  (3.2 + )

Re: [PATCH] rcu: restore correct batch limiting

[Paul E. McKenney]

On Thu, Oct 18, 2012 at 02:44:50PM +0200, Eric Dumazet wrote:
> On Thu, 2012-10-18 at 04:58 -0700, Paul E. McKenney wrote:
> > On Wed, Oct 17, 2012 at 09:14:12AM +0200, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > > 
> > > Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> > > invocation) added a regression in rcu_do_batch()
> > > 
> > > Under stress, RCU is supposed to allow to process all items in queue,
> > > instead of a batch of 10 items (blimit), but an integer overflow makes
> > > the effective limit being 1.
> > > 
> > > So RCU cannot recover and machine eventually crash because of OOM.
> > > 
> > > Using long instead of int is not really needed, convert everything
> > > to integers.
> > 
> > <facepalm>
> > 
> > Good catch!!!
> > 
> > The reason for favoring long over int is that there are a few systems out
> > there with terabytes of main memory.  In addition, there have been a few
> > bugs over the past few years that could result in RCU CPU stalls of more
> > than a minute.  This makes it impossible to rule out the possibility of
> > a billion callbacks appearing on one CPU.
> > 
> > So, does the following patch fix things, or a I still confused?
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > rcu: Fix batch-limit size problem
> > 
> > Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
> > invocation) added a regression in rcu_do_batch()
> > 
> > Under stress, RCU is supposed to allow to process all items in queue,
> > instead of a batch of 10 items (blimit), but an integer overflow makes
> > the effective limit being 1.  So, unless there is frequent idle periods
> > (during which RCU ignores batch limits), RCU can be forced into a
> > state where it cannot keep up with the callback-generation rate,
> > eventually resulting in OOM.
> > 
> > This commit therefore converts a few variables in rcu_do_batch() from
> > int to long to fix this problem.
> > 
> > Reported-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index e36d085..e056e1e 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1823,7 +1823,8 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
> >  {
> >  	unsigned long flags;
> >  	struct rcu_head *next, *list, **tail;
> > -	int bl, count, count_lazy, i;
> > +	long bl, count, count_lazy;
> > +	int i;
> >  
> >  	/* If no callbacks are ready, just return.*/
> >  	if (!cpu_has_callbacks_ready_to_invoke(rdp)) {
> > 
> 
> Yes, why not, but global "int blimit" being an int, I really dont see
> the point having a long in struct rcu_data.
> 
> Having 2 billions callbacks on one cpu would be problematic, I really
> hope nobody relies on this ;)

Fair point!  ;-)

But just making everything long makes it quite easy to analyze.

> I guess the 10/infinity switch should be smarter.
> 
> something like the following maybe :
> 
> rdp->blimit = max(blimit, rdp->qlen >> 6);
> 
> (if queue is big, dont wait to hit 10000 before allowing more items to
> be handled per round)

The -rt guys would not be amused.  :-(

But for non-realtime use, increasing rcutree.blimit either at boot or
via sysfs could make sense.  It is also likely that I will move callback
processing to a kthread at some point, which would allow some additional
flexibility.

Furthermore, it would be easy to have one default for non-rt and another
for -rt, if that would help.

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Please dont forget stable teams.  (3.2 + )

Added both, please see below!

							Thanx, Paul

------------------------------------------------------------------------

rcu: Fix batch-limit size problem

Commit 29c00b4a1d9e27 (rcu: Add event-tracing for RCU callback
invocation) added a regression in rcu_do_batch()

Under stress, RCU is supposed to allow to process all items in queue,
instead of a batch of 10 items (blimit), but an integer overflow makes
the effective limit being 1.  So, unless there is frequent idle periods
(during which RCU ignores batch limits), RCU can be forced into a
state where it cannot keep up with the callback-generation rate,
eventually resulting in OOM.

This commit therefore converts a few variables in rcu_do_batch() from
int to long to fix this problem, along with the module parameters
controlling the batch limits.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org> # 3.2 +

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e36d085..3a7170b 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -212,13 +212,13 @@ DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
 #endif
 };
 
-static int blimit = 10;		/* Maximum callbacks per rcu_do_batch. */
-static int qhimark = 10000;	/* If this many pending, ignore blimit. */
-static int qlowmark = 100;	/* Once only this many pending, use blimit. */
+static long blimit = 10;	/* Maximum callbacks per rcu_do_batch. */
+static long qhimark = 10000;	/* If this many pending, ignore blimit. */
+static long qlowmark = 100;	/* Once only this many pending, use blimit. */
 
-module_param(blimit, int, 0444);
-module_param(qhimark, int, 0444);
-module_param(qlowmark, int, 0444);
+module_param(blimit, long, 0444);
+module_param(qhimark, long, 0444);
+module_param(qlowmark, long, 0444);
 
 int rcu_cpu_stall_suppress __read_mostly; /* 1 = suppress stall warnings. */
 int rcu_cpu_stall_timeout __read_mostly = CONFIG_RCU_CPU_STALL_TIMEOUT;
@@ -1823,7 +1823,8 @@ static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp)
 {
 	unsigned long flags;
 	struct rcu_head *next, *list, **tail;
-	int bl, count, count_lazy, i;
+	long bl, count, count_lazy;
+	int i;
 
 	/* If no callbacks are ready, just return.*/
 	if (!cpu_has_callbacks_ready_to_invoke(rdp)) {

Re: [PATCH] rcu: restore correct batch limiting

[Eric Dumazet]

On Thu, 2012-10-18 at 08:25 -0700, Paul E. McKenney wrote:
> On Thu, Oct 18, 2012 at 02:44:50PM +0200, Eric Dumazet wrote:

> > 
> > Having 2 billions callbacks on one cpu would be problematic, I really
> > hope nobody relies on this ;)
> 
> Fair point!  ;-)
> 
> But just making everything long makes it quite easy to analyze.
> 
> > I guess the 10/infinity switch should be smarter.
> > 
> > something like the following maybe :
> > 
> > rdp->blimit = max(blimit, rdp->qlen >> 6);
> > 
> > (if queue is big, dont wait to hit 10000 before allowing more items to
> > be handled per round)
> 
> The -rt guys would not be amused.  :-(
> 
> But for non-realtime use, increasing rcutree.blimit either at boot or
> via sysfs could make sense.  It is also likely that I will move callback
> processing to a kthread at some point, which would allow some additional
> flexibility.
> 

Ah, I now realize the loop can exceed blimit, but is it true for BH
variant ? (Not really a problem for 3.6/3.7 kernels, but prior ones)

if (++count >= bl &&
    (need_resched() ||
     (!is_idle_task(current) && !rcu_is_callbacks_kthread())))
        break;

I wonder if ksoftirqd should be included as well...

> Furthermore, it would be easy to have one default for non-rt and another
> for -rt, if that would help.
> 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > 
> > Please dont forget stable teams.  (3.2 + )
> 
> Added both, please see below!
> 

Seems fine to me, thanks Paul !

Re: [PATCH] rcu: restore correct batch limiting

[Paul E. McKenney]

On Thu, Oct 18, 2012 at 06:10:28PM +0200, Eric Dumazet wrote:
> On Thu, 2012-10-18 at 08:25 -0700, Paul E. McKenney wrote:
> > On Thu, Oct 18, 2012 at 02:44:50PM +0200, Eric Dumazet wrote:
> 
> > > 
> > > Having 2 billions callbacks on one cpu would be problematic, I really
> > > hope nobody relies on this ;)
> > 
> > Fair point!  ;-)
> > 
> > But just making everything long makes it quite easy to analyze.
> > 
> > > I guess the 10/infinity switch should be smarter.
> > > 
> > > something like the following maybe :
> > > 
> > > rdp->blimit = max(blimit, rdp->qlen >> 6);
> > > 
> > > (if queue is big, dont wait to hit 10000 before allowing more items to
> > > be handled per round)
> > 
> > The -rt guys would not be amused.  :-(
> > 
> > But for non-realtime use, increasing rcutree.blimit either at boot or
> > via sysfs could make sense.  It is also likely that I will move callback
> > processing to a kthread at some point, which would allow some additional
> > flexibility.
> > 
> 
> Ah, I now realize the loop can exceed blimit, but is it true for BH
> variant ? (Not really a problem for 3.6/3.7 kernels, but prior ones)

Yep, applies to all the RCU flavors.

> if (++count >= bl &&
>     (need_resched() ||
>      (!is_idle_task(current) && !rcu_is_callbacks_kthread())))
>         break;
> 
> I wonder if ksoftirqd should be included as well...

This would be safe only if ksoftirqd could be guaranteed to be the
lowest-priority process on the given CPU, which I do not believe to be
the case.  The problem is that if ksoftirqd is not the lowest-priority
process on the given CPU, we can seriously delay that other process
for no good reason.  The fact that ksoftirqd does local_bh_disable()
means that the scheduler cannot preempt it, either.  :-(

> > Furthermore, it would be easy to have one default for non-rt and another
> > for -rt, if that would help.
> > 
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > 
> > > Please dont forget stable teams.  (3.2 + )
> > 
> > Added both, please see below!
> 
> Seems fine to me, thanks Paul !

Thank you for everything on this one!

							Thanx, Paul