linux-kernel - Re: [PATCH] rcu: remove the context test in rcu_read_unlock

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 Mar 2012 14:56:13 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rcu: remove the context test in rcu_read_unlock_special()

On Thu, Mar 29, 2012 at 04:21:44PM +0800, Lai Jiangshan wrote:
> Because of commit#26861f and #10f39bb1b2, the context test in
> rcu_read_unlock_special() are not needed.
> 
> commit#26861f and #10f39bb1b2 ensure ->rcu_read_unlock_special is zero
> after rcu_read_unlock(), which implies:
> 
> if ->rcu_read_unlock_special is non-zero, the out-most RCU C.S. can
> be interrupted or can be preempted, such context is safe to
> call rcu_read_unlock_special().

In theory, I am very much in favor of this, but in practice we need to
put a WARN_ON_ONCE() in the "if" statement to be removed for a few years
first.  If there are never any warnings (or, more likely, once such
warnings have been fixed for some time), then this patch below would be
eminently sensible.

							Thanx, Paul

> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> ---
> diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
> index 22ecea0..22e2ef1 100644
> --- a/kernel/rcutiny_plugin.h
> +++ b/kernel/rcutiny_plugin.h
> @@ -571,12 +571,6 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
>  	if (special & RCU_READ_UNLOCK_NEED_QS)
>  		rcu_preempt_cpu_qs();
> 
> -	/* Hardware IRQ handlers cannot block. */
> -	if (in_irq() || in_serving_softirq()) {
> -		local_irq_restore(flags);
> -		return;
> -	}
> -
>  	/* Clean up if blocked during RCU read-side critical section. */
>  	if (special & RCU_READ_UNLOCK_BLOCKED) {
>  		t->rcu_read_unlock_special &= ~RCU_READ_UNLOCK_BLOCKED;
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index c023464..2bd3689 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -338,12 +338,6 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
>  		rcu_preempt_qs(smp_processor_id());
>  	}
> 
> -	/* Hardware IRQ handlers cannot block. */
> -	if (in_irq() || in_serving_softirq()) {
> -		local_irq_restore(flags);
> -		return;
> -	}
> -
>  	/* Clean up if blocked during RCU read-side critical section. */
>  	if (special & RCU_READ_UNLOCK_BLOCKED) {
>  		t->rcu_read_unlock_special &= ~RCU_READ_UNLOCK_BLOCKED;
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 671f959..7074f97 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -318,12 +318,8 @@ static inline void invoke_softirq(void)
>  #else
>  		do_softirq();
>  #endif
> -	} else {
> -		__local_bh_disable((unsigned long)__builtin_return_address(0),
> -				SOFTIRQ_OFFSET);
> +	} else
>  		wakeup_softirqd();
> -		__local_bh_enable(SOFTIRQ_OFFSET);
> -	}
>  }
> 
>  /*
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/