| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 24 Nov 2015 23:52:12 +0800
From: Boqun Feng <boqun.feng@...il.com>
To: Daniel Wagner <daniel.wagner@...-carit.de>
Cc: linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marcelo Tosatti <mtosatti@...hat.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: Re: [PATCH tip v4 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while
holding rnp->lock
Hi Daniel,
On Tue, Nov 24, 2015 at 02:03:06PM +0100, Daniel Wagner wrote:
> rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
> this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
> not enable the IRQs. lockdep is happy.
>
> By switching over using swait this is not true anymore. swake_up_all()
> enables the IRQs while processing the waiters. __do_softirq() can now
> run and will eventually call rcu_process_callbacks() which wants to
> grap nrp->lock.
>
> Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
> switch over to swait.
>
But you did introduce swait in this patch ;-)
[snip]
>
> Signed-off-by: Daniel Wagner <daniel.wagner@...-carit.de>
> Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: linux-kernel@...r.kernel.org
> ---
> kernel/rcu/tree.c | 4 +++-
> kernel/rcu/tree.h | 3 ++-
> kernel/rcu/tree_plugin.h | 16 +++++++++++++---
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
So I tried to build this patch with a config having RCU_EXPERT=y and
RCU_NOCB_CPU=y, but I got:
In file included from include/linux/completion.h:11:0,
from include/linux/rcupdate.h:43,
from include/linux/sysctl.h:25,
from include/linux/timer.h:242,
from include/linux/workqueue.h:8,
from include/linux/pm.h:25,
from ./arch/x86/include/asm/apic.h:5,
from ./arch/x86/include/asm/smp.h:12,
from include/linux/smp.h:59,
from kernel/rcu/tree.c:34:
kernel/rcu/tree_plugin.h: In function ‘rcu_nocb_gp_cleanup’:
kernel/rcu/tree_plugin.h:1782:14: warning: passing argument 1 of ‘__wake_up’ from incompatible pointer type [-Wincompatible-pointer-types]
wake_up_all(sq);
^
include/linux/wait.h:168:36: note: in definition of macro ‘wake_up_all’
#define wake_up_all(x) __wake_up(x, TASK_NORMAL, 0, NULL)
(I also attach the configure file in case you need it)
I think the reason is that you introduced swait in this patch, but
you didn't use the proper function, e.g. swake_up_all(), and also you
didn't #include <linux/swait.h> ;-)
Regards,
Boqun
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 775d36c..952536d 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1568,7 +1568,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> int needmore;
> struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
>
> - rcu_nocb_gp_cleanup(rsp, rnp);
> rnp->need_future_gp[c & 0x1] = 0;
> needmore = rnp->need_future_gp[(c + 1) & 0x1];
> trace_rcu_future_gp(rnp, rdp, c,
> @@ -1972,6 +1971,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> int nocb = 0;
> struct rcu_data *rdp;
> struct rcu_node *rnp = rcu_get_root(rsp);
> + struct swait_queue_head *sq;
>
> WRITE_ONCE(rsp->gp_activity, jiffies);
> raw_spin_lock_irq(&rnp->lock);
> @@ -2010,7 +2010,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
> /* smp_mb() provided by prior unlock-lock pair. */
> nocb += rcu_future_gp_cleanup(rsp, rnp);
> + sq = rcu_nocb_gp_get(rnp);
> raw_spin_unlock_irq(&rnp->lock);
> + rcu_nocb_gp_cleanup(sq);
> cond_resched_rcu_qs();
> WRITE_ONCE(rsp->gp_activity, jiffies);
> rcu_gp_slow(rsp, gp_cleanup_delay);
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index 2e991f8..3dcf6368 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -608,7 +608,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
> static void increment_cpu_stall_ticks(void);
> static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
> static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp);
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq);
> static void rcu_init_one_nocb(struct rcu_node *rnp);
> static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
> bool lazy, unsigned long flags);
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index b2bf396..db4f357 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1777,9 +1777,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
> * Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
> * grace period.
> */
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
> {
> - wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
> + wake_up_all(sq);
> }
>
> /*
> @@ -1795,6 +1795,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
> rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
> }
>
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
> +{
> + return &rnp->nocb_gp_wq[rnp->completed & 0x1];
> +}
> +
> static void rcu_init_one_nocb(struct rcu_node *rnp)
> {
> init_waitqueue_head(&rnp->nocb_gp_wq[0]);
> @@ -2469,7 +2474,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
> return false;
> }
>
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
> {
> }
>
> @@ -2477,6 +2482,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
> {
> }
>
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
> +{
> + return NULL;
> +}
> +
> static void rcu_init_one_nocb(struct rcu_node *rnp)
> {
> }
> --
> 2.4.3
>
View attachment "swait.config" of type "text/plain" (101926 bytes)
Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)
Powered by blists - more mailing lists