Subject: Unnecessary overhead with stack protector. - msg#07064

Previous Message by Date: click to view message preview

Re: [PATCH tip/core/rcu 2/6] rcu: prevent RCU IPI storms in presence of high call_rcu() load

On Thu, Oct 15, 2009 at 01:04:04PM +0200, Nick Piggin wrote: > Testing this on top of my vfs-scale patches, a 64-way 32-node ia64 > system runs the parallel open/close microbenchmark ~30 times faster > and is scaling linearly now. Single thread performance is not > noticably changed. Thanks very much Paul. Good to hear, thank you for giving it a go! Thanx, Paul > On Wed, Oct 14, 2009 at 10:15:55AM -0700, Paul E. McKenney wrote: > > From: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> > > > > As the number of callbacks on a given CPU rises, invoke > > force_quiescent_state() only every blimit number of callbacks > > (defaults to 10,000), and even then only if no other CPU has invoked > > force_quiescent_state() in the meantime. > > > > Reported-by: Nick Piggin <npiggin@xxxxxxx> > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> > > --- > > kernel/rcutree.c | 29 ++++++++++++++++++++++++----- > > kernel/rcutree.h | 4 ++++ > > 2 files changed, 28 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c > > index 705f02a..ddbf111 100644 > > --- a/kernel/rcutree.c > > +++ b/kernel/rcutree.c > > @@ -958,7 +958,7 @@ static void rcu_offline_cpu(int cpu) > > * Invoke any RCU callbacks that have made it to the end of their grace > > * period. Thottle as specified by rdp->blimit. > > */ > > -static void rcu_do_batch(struct rcu_data *rdp) > > +static void rcu_do_batch(struct rcu_state *rsp, struct rcu_data *rdp) > > { > > unsigned long flags; > > struct rcu_head *next, *list, **tail; > > @@ -1011,6 +1011,13 @@ static void rcu_do_batch(struct rcu_data *rdp) > > if (rdp->blimit == LONG_MAX && rdp->qlen <= qlowmark) > > rdp->blimit = blimit; > > > > + /* Reset ->qlen_last_fqs_check trigger if enough CBs have drained. */ > > + if (rdp->qlen == 0 && rdp->qlen_last_fqs_check != 0) { > > + rdp->qlen_last_fqs_check = 0; > > + rdp->n_force_qs_snap = rsp->n_force_qs; > > + } else if (rdp->qlen < rdp->qlen_last_fqs_check - qhimark) > > + rdp->qlen_last_fqs_check = rdp->qlen; > > + > > local_irq_restore(flags); > > > > /* Re-raise the RCU softirq if there are callbacks remaining. */ > > @@ -1224,7 +1231,7 @@ __rcu_process_callbacks(struct rcu_state *rsp, struct > > rcu_data *rdp) > > } > > > > /* If there are callbacks ready, invoke them. */ > > - rcu_do_batch(rdp); > > + rcu_do_batch(rsp, rdp); > > } > > > > /* > > @@ -1288,10 +1295,20 @@ __call_rcu(struct rcu_head *head, void > > (*func)(struct rcu_head *rcu), > > rcu_start_gp(rsp, nestflag); /* releases rnp_root->lock. */ > > } > > > > - /* Force the grace period if too many callbacks or too long waiting. */ > > - if (unlikely(++rdp->qlen > qhimark)) { > > + /* > > + * Force the grace period if too many callbacks or too long waiting. > > + * Enforce hysteresis, and don't invoke force_quiescent_state() > > + * if some other CPU has recently done so. Also, don't bother > > + * invoking force_quiescent_state() if the newly enqueued callback > > + * is the only one waiting for a grace period to complete. > > + */ > > + if (unlikely(++rdp->qlen > rdp->qlen_last_fqs_check + qhimark)) { > > rdp->blimit = LONG_MAX; > > - force_quiescent_state(rsp, 0); > > + if (rsp->n_force_qs == rdp->n_force_qs_snap && > > + *rdp->nxttail[RCU_DONE_TAIL] != head) > > + force_quiescent_state(rsp, 0); > > + rdp->n_force_qs_snap = rsp->n_force_qs; > > + rdp->qlen_last_fqs_check = rdp->qlen; > > } else if ((long)(ACCESS_ONCE(rsp->jiffies_force_qs) - jiffies) < 0) > > force_quiescent_state(rsp, 1); > > local_irq_restore(flags); > > @@ -1523,6 +1540,8 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp, > > int preemptable) > > rdp->beenonline = 1; /* We have now been online. */ > > rdp->preemptable = preemptable; > > rdp->passed_quiesc_completed = lastcomp - 1; > > + rdp->qlen_last_fqs_check = 0; > > + rdp->n_force_qs_snap = rsp->n_force_qs; > > rdp->blimit = blimit; > > spin_unlock(&rnp->lock); /* irqs remain disabled. */ > > > > diff --git a/kernel/rcutree.h b/kernel/rcutree.h > > index b40ac57..599161f 100644 > > --- a/kernel/rcutree.h > > +++ b/kernel/rcutree.h > > @@ -167,6 +167,10 @@ struct rcu_data { > > struct rcu_head *nxtlist; > > struct rcu_head **nxttail[RCU_NEXT_SIZE]; > > long qlen; /* # of queued callbacks */ > > + long qlen_last_fqs_check; > > + /* qlen at last check for QS forcing */ > > + unsigned long n_force_qs_snap; > > + /* did other CPU force QS recently? */ > > long blimit; /* Upper limit on a processed batch */ > > > > #ifdef CONFIG_NO_HZ > > -- > > 1.5.2.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Next Message by Date: click to view message preview

[tip:core/urgent] rcu: Fix TREE_PREEMPT_RCU CPU_HOTPLUG bad-luck hang

Commit-ID: 237c80c5c8fb7ec128cf2a756b550dc41ad7eac7 Gitweb: http://git.kernel.org/tip/237c80c5c8fb7ec128cf2a756b550dc41ad7eac7 Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> AuthorDate: Thu, 15 Oct 2009 09:26:14 -0700 Committer: Ingo Molnar <mingo@xxxxxxx> CommitDate: Thu, 15 Oct 2009 20:33:01 +0200 rcu: Fix TREE_PREEMPT_RCU CPU_HOTPLUG bad-luck hang If the following sequence of events occurs, then TREE_PREEMPT_RCU will hang waiting for a grace period to complete, eventually OOMing the system: o A TREE_PREEMPT_RCU build of the kernel is booted on a system with more than 64 physical CPUs present (32 on a 32-bit system). Alternatively, a TREE_PREEMPT_RCU build of the kernel is booted with RCU_FANOUT set to a sufficiently small value that the physical CPUs populate two or more leaf rcu_node structures. o A task is preempted in an RCU read-side critical section while running on a CPU corresponding to a given leaf rcu_node structure. o All CPUs corresponding to this same leaf rcu_node structure record quiescent states for the current grace period. o All of these same CPUs go offline (hence the need for enough physical CPUs to populate more than one leaf rcu_node structure). This causes the preempted task to be moved to the root rcu_node structure. At this point, there is nothing left to cause the quiescent state to be propagated up the rcu_node tree, so the current grace period never completes. The simplest fix, especially after considering the deadlock possibilities, is to detect this situation when the last CPU is offlined, and to set that CPU's ->qsmask bit in its leaf rcu_node structure. This will cause the next invocation of force_quiescent_state() to end the grace period. Without this fix, this hang can be triggered in an hour or so on some machines with rcutorture and random CPU onlining/offlining. With this fix, these same machines pass a full 10 hours of this sort of abuse. Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> Cc: laijs@xxxxxxxxxxxxxx Cc: dipankar@xxxxxxxxxx Cc: mathieu.desnoyers@xxxxxxxxxx Cc: josh@xxxxxxxxxxxxxxxx Cc: dvhltc@xxxxxxxxxx Cc: niv@xxxxxxxxxx Cc: peterz@xxxxxxxxxxxxx Cc: rostedt@xxxxxxxxxxx Cc: Valdis.Kletnieks@xxxxxx Cc: dhowells@xxxxxxxxxx LKML-Reference: <20091015162614.GA19131@xxxxxxxxxxxxxxxxxx> Signed-off-by: Ingo Molnar <mingo@xxxxxxx> --- kernel/rcutree.c | 15 ++++++++++++++- kernel/rcutree.h | 6 +++--- kernel/rcutree_plugin.h | 25 +++++++++++++++++-------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/kernel/rcutree.c b/kernel/rcutree.c index ddbf111..0536125 100644 --- a/kernel/rcutree.c +++ b/kernel/rcutree.c @@ -913,7 +913,20 @@ static void __rcu_offline_cpu(int cpu, struct rcu_state *rsp) spin_unlock(&rnp->lock); /* irqs remain disabled. */ break; } - rcu_preempt_offline_tasks(rsp, rnp, rdp); + + /* + * If there was a task blocking the current grace period, + * and if all CPUs have checked in, we need to propagate + * the quiescent state up the rcu_node hierarchy. But that + * is inconvenient at the moment due to deadlock issues if + * this should end the current grace period. So set the + * offlined CPU's bit in ->qsmask in order to force the + * next force_quiescent_state() invocation to clean up this + * mess in a deadlock-free manner. + */ + if (rcu_preempt_offline_tasks(rsp, rnp, rdp) && !rnp->qsmask) + rnp->qsmask |= mask; + mask = rnp->grpmask; spin_unlock(&rnp->lock); /* irqs remain disabled. */ rnp = rnp->parent; diff --git a/kernel/rcutree.h b/kernel/rcutree.h index 599161f..1823c6e 100644 --- a/kernel/rcutree.h +++ b/kernel/rcutree.h @@ -306,9 +306,9 @@ static void rcu_print_task_stall(struct rcu_node *rnp); #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp); #ifdef CONFIG_HOTPLUG_CPU -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp); +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp); static void rcu_preempt_offline_cpu(int cpu); #endif /* #ifdef CONFIG_HOTPLUG_CPU */ static void rcu_preempt_check_callbacks(int cpu); diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h index ebd20ee..ef2a58c 100644 --- a/kernel/rcutree_plugin.h +++ b/kernel/rcutree_plugin.h @@ -304,21 +304,25 @@ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp) * parent is to remove the need for rcu_read_unlock_special() to * make more than two attempts to acquire the target rcu_node's lock. * + * Returns 1 if there was previously a task blocking the current grace + * period on the specified rcu_node structure. + * * The caller must hold rnp->lock with irqs disabled. */ -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp) +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp) { int i; struct list_head *lp; struct list_head *lp_root; + int retval = rcu_preempted_readers(rnp); struct rcu_node *rnp_root = rcu_get_root(rsp); struct task_struct *tp; if (rnp == rnp_root) { WARN_ONCE(1, "Last CPU thought to be offlined?"); - return; /* Shouldn't happen: at least one CPU online. */ + return 0; /* Shouldn't happen: at least one CPU online. */ } WARN_ON_ONCE(rnp != rdp->mynode && (!list_empty(&rnp->blocked_tasks[0]) || @@ -342,6 +346,8 @@ static void rcu_preempt_offline_tasks(struct rcu_state *rsp, spin_unlock(&rnp_root->lock); /* irqs remain disabled */ } } + + return retval; } /* @@ -532,12 +538,15 @@ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp) /* * Because preemptable RCU does not exist, it never needs to migrate - * tasks that were blocked within RCU read-side critical sections. + * tasks that were blocked within RCU read-side critical sections, and + * such non-existent tasks cannot possibly have been blocking the current + * grace period. */ -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp) +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp) { + return 0; } /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Previous Message by Thread: click to view message preview

[PATCH tip/core/rcu] rcu: fix TREE_PREEMPT_RCU CPU_HOTPLUG bad-luck hang

If the following sequence of events occurs, then TREE_PREEMPT_RCU will hang waiting for a grace period to complete, eventually OOMing the system: o A TREE_PREEMPT_RCU build of the kernel is booted on a system with more than 64 physical CPUs present (32 on a 32-bit system). Alternatively, a TREE_PREEMPT_RCU build of the kernel is booted with RCU_FANOUT set to a sufficiently small value that the physical CPUs populate two or more leaf rcu_node structures. o A task is preempted in an RCU read-side critical section while running on a CPU corresponding to a given leaf rcu_node structure. o All CPUs corresponding to this same leaf rcu_node structure record quiescent states for the current grace period. o All of these same CPUs go offline (hence the need for enough physical CPUs to populate more than one leaf rcu_node structure). This causes the preempted task to be moved to the root rcu_node structure. At this point, there is nothing left to cause the quiescent state to be propagated up the rcu_node tree, so the current grace period never completes. The simplest fix, especially after considering the deadlock possibilities, is to detect this situation when the last CPU is offlined, and to set that CPU's ->qsmask bit in its leaf rcu_node structure. This will cause the next invocation of force_quiescent_state() to end the grace period. Without this fix, this hang can be triggered in an hour or so on some machines with rcutorture and random CPU onlining/offlining. With this fix, these same machines pass a full 10 hours of this sort of abuse. Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> --- kernel/rcutree.c | 15 ++++++++++++++- kernel/rcutree.h | 6 +++--- kernel/rcutree_plugin.h | 25 +++++++++++++++++-------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/kernel/rcutree.c b/kernel/rcutree.c index ddbf111..0536125 100644 --- a/kernel/rcutree.c +++ b/kernel/rcutree.c @@ -913,7 +913,20 @@ static void __rcu_offline_cpu(int cpu, struct rcu_state *rsp) spin_unlock(&rnp->lock); /* irqs remain disabled. */ break; } - rcu_preempt_offline_tasks(rsp, rnp, rdp); + + /* + * If there was a task blocking the current grace period, + * and if all CPUs have checked in, we need to propagate + * the quiescent state up the rcu_node hierarchy. But that + * is inconvenient at the moment due to deadlock issues if + * this should end the current grace period. So set the + * offlined CPU's bit in ->qsmask in order to force the + * next force_quiescent_state() invocation to clean up this + * mess in a deadlock-free manner. + */ + if (rcu_preempt_offline_tasks(rsp, rnp, rdp) && !rnp->qsmask) + rnp->qsmask |= mask; + mask = rnp->grpmask; spin_unlock(&rnp->lock); /* irqs remain disabled. */ rnp = rnp->parent; diff --git a/kernel/rcutree.h b/kernel/rcutree.h index 599161f..1823c6e 100644 --- a/kernel/rcutree.h +++ b/kernel/rcutree.h @@ -306,9 +306,9 @@ static void rcu_print_task_stall(struct rcu_node *rnp); #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp); #ifdef CONFIG_HOTPLUG_CPU -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp); +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp); static void rcu_preempt_offline_cpu(int cpu); #endif /* #ifdef CONFIG_HOTPLUG_CPU */ static void rcu_preempt_check_callbacks(int cpu); diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h index ebd20ee..ef2a58c 100644 --- a/kernel/rcutree_plugin.h +++ b/kernel/rcutree_plugin.h @@ -304,21 +304,25 @@ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp) * parent is to remove the need for rcu_read_unlock_special() to * make more than two attempts to acquire the target rcu_node's lock. * + * Returns 1 if there was previously a task blocking the current grace + * period on the specified rcu_node structure. + * * The caller must hold rnp->lock with irqs disabled. */ -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp) +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp) { int i; struct list_head *lp; struct list_head *lp_root; + int retval = rcu_preempted_readers(rnp); struct rcu_node *rnp_root = rcu_get_root(rsp); struct task_struct *tp; if (rnp == rnp_root) { WARN_ONCE(1, "Last CPU thought to be offlined?"); - return; /* Shouldn't happen: at least one CPU online. */ + return 0; /* Shouldn't happen: at least one CPU online. */ } WARN_ON_ONCE(rnp != rdp->mynode && (!list_empty(&rnp->blocked_tasks[0]) || @@ -342,6 +346,8 @@ static void rcu_preempt_offline_tasks(struct rcu_state *rsp, spin_unlock(&rnp_root->lock); /* irqs remain disabled */ } } + + return retval; } /* @@ -532,12 +538,15 @@ static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp) /* * Because preemptable RCU does not exist, it never needs to migrate - * tasks that were blocked within RCU read-side critical sections. + * tasks that were blocked within RCU read-side critical sections, and + * such non-existent tasks cannot possibly have been blocking the current + * grace period. */ -static void rcu_preempt_offline_tasks(struct rcu_state *rsp, - struct rcu_node *rnp, - struct rcu_data *rdp) +static int rcu_preempt_offline_tasks(struct rcu_state *rsp, + struct rcu_node *rnp, + struct rcu_data *rdp) { + return 0; } /* -- 1.5.2.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Next Message by Thread: click to view message preview

Re: Unnecessary overhead with stack protector.

(Cc:-ed Arjan too.) * Dave Jones <davej@xxxxxxxxxx> wrote: > 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that made > CC_STACKPROTECTOR_ALL not-selectable if someone enables > CC_STACKPROTECTOR. > > We've noticed in Fedora that this has introduced noticable overhead on > some functions, including those which don't even have any on-stack > variables. > > According to the gcc manpage, -fstack-protector will protect functions > with as little as 8 bytes of stack usage. So we're introducing a huge > amount of overhead, to close a small amount of vulnerability (the >0 > && <8 case). > > The overhead as it stands right now means this whole option is > unusable for a distro kernel without reverting the above commit. Exactly what workload showed overhead, and how much? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/