Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> 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