Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Jürgen Groß" <jgross@suse.com>,
	"Igor Druzhinin" <igor.druzhinin@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()
Date: Fri, 27 Mar 2020 09:35:24 +0100
Message-ID: <859b4b9e-d839-0961-6c09-4c6aebefe9e4@suse.com> (raw)
In-Reply-To: <7a9cff0b-4c8f-899a-3fae-8a703bc90125@suse.com>

On 27.03.2020 09:10, Jürgen Groß wrote:
> On 27.03.20 00:24, Igor Druzhinin wrote:
>> On 26/03/2020 09:19, Juergen Gross wrote:
>>> Some keyhandlers are calling process_pending_softirqs() while holding
>>> a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
>>> activate rcu calls which should not happen inside a rcu_read_lock().
>>>
>>> For that purpose modify process_pending_softirqs() to not allow rcu
>>> callback processing when a rcu_read_lock() is being held.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> V3:
>>> - add RCU_SOFTIRQ to ignore in process_pending_softirqs_norcu()
>>>    (Roger Pau Monné)
>>>
>>> V5:
>>> - block rcu processing depending on rch_read_lock() being held or not
>>>    (Jan Beulich)
>>
>> Juergen,
>>
>> Our BVT revealed a likely problem with this commit in that form.
>> Since 12509bbeb9e ("rwlocks: call preempt_disable() when taking a rwlock")
>> preemption is disabled after taking cpu_maps which will block RCU
>> callback processing inside rcu_barrier itself. This will result in
> 
> Why would that block RCU callback processing?
> 
> RCU callbacks should be blocked only if a rcu lock is being held.
> 
> Did I miss something in my patches?

Igor, are you perhaps running without "rcu: add assertions to debug
build"? I think this actually fixes what you describe. Without it
rcu_barrier(), in its second loop, calling process_pending_softirqs(),
would cause the RCU softirq to not be invoked anymore with preemption
disabled. Of course the title of this change doesn't reflect this at
all.

Jürgen, as an aside, while looking at the code again, I think the
comment near the end of process_pending_softirqs() would now rather
belong at its very beginning; should have spotted this while
reviewing.

Jan


  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26  9:19 [Xen-devel] [PATCH v8 0/5] xen/rcu: let rcu work better with core scheduling Juergen Gross
2020-03-26  9:19 ` [Xen-devel] [PATCH v8 1/5] xen: introduce smp_mb__[after|before]_atomic() barriers Juergen Gross
2020-03-26  9:19 ` [Xen-devel] [PATCH v8 2/5] xen/rcu: don't use stop_machine_run() for rcu_barrier() Juergen Gross
2020-03-26  9:19 ` [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock() Juergen Gross
2020-03-26 23:24   ` Igor Druzhinin
2020-03-27  8:10     ` Jürgen Groß
2020-03-27  8:35       ` Jan Beulich [this message]
2020-03-27  8:39         ` Jürgen Groß
2020-03-27  9:56         ` Igor Druzhinin
2020-03-26  9:19 ` [Xen-devel] [PATCH v8 4/5] xen/rcu: add assertions to debug build Juergen Gross
2020-03-26  9:19 ` [Xen-devel] [PATCH v8 5/5] xen/rcu: add per-lock counter in debug builds Juergen Gross

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=859b4b9e-d839-0961-6c09-4c6aebefe9e4@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=igor.druzhinin@citrix.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git