All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: paulmck@linux.vnet.ibm.com
Cc: Ingo Molnar <mingo@elte.hu>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -tip] rcu: local_irq_disable() also delimits RCU_SCHED read-site critical sections
Date: Wed, 17 Mar 2010 09:41:55 +0800	[thread overview]
Message-ID: <4BA03363.8040604@cn.fujitsu.com> (raw)
In-Reply-To: <20100316132156.GC6709@linux.vnet.ibm.com>

Paul E. McKenney wrote:
> On Tue, Mar 16, 2010 at 07:09:21PM +0800, Lai Jiangshan wrote:
>> It is documented that local_irq_disable() also delimits
>> RCU_SCHED read-site critical sections.
>> See the document of synchronize_sched() or
>> Documentation/RCU/whatisRCU.txt.
>>
>> So we have to test irqs_disabled() in rcu_read_lock_sched_held().
>> Otherwise rcu-lockdep brings incorrect complaint.
> 
> Interesting -- I was under the impression that preempt_count() covered
> this as well, due to the following in include/linux/hardirq.h:
> 
> 	#define PREEMPT_MASK    (__IRQ_MASK(PREEMPT_BITS) << PREEMPT_SHIFT)
> 	#define SOFTIRQ_MASK    (__IRQ_MASK(SOFTIRQ_BITS) << SOFTIRQ_SHIFT)
> 	#define HARDIRQ_MASK    (__IRQ_MASK(HARDIRQ_BITS) << HARDIRQ_SHIFT)
> 	#define NMI_MASK        (__IRQ_MASK(NMI_BITS)     << NMI_SHIFT)
> 
> But irqs_disabled() does look to sample the actual interrupt hardware.
> 
> So, if there are cases where RCU is used where the interrupt hardware
> is disabled, but preempt_count() has not been updated, this patch is
> the right thing to do.
> 
> 							Thanx, Paul
> 

local_irq_disable() does not touch preempt_count, it touchs
a register of current CPU.

The following quick test module is a case where RCU_SCHED is used where
the interrupt hardware is disabled, but preempt_count() has
not been updated, and it raises rcu-lockdep complaint.

#include <linux/module.h>
#include <linux/hardirq.h>
#include <linux/rcupdate.h>

void *test = &test;

int test_init(void)
{
	local_irq_disable();
	printk(KERN_INFO "%p\n", rcu_dereference_sched(test));
	local_irq_enable();

	return 0;
}

void test_exit(void) {}

module_init(test_init);
module_exit(test_exit);
MODULE_LICENSE("GPL");

  reply	other threads:[~2010-03-17  1:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16 11:09 [PATCH -tip] rcu: local_irq_disable() also delimits RCU_SCHED read-site critical sections Lai Jiangshan
2010-03-16 11:28 ` Ingo Molnar
2010-03-16 13:21 ` Paul E. McKenney
2010-03-17  1:41   ` Lai Jiangshan [this message]
2010-03-17  2:26   ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BA03363.8040604@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.