lkml.org 
[lkml]   [2018]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 5/7] block: Remove superflous rcu_read_[un]lock_sched() in blk_queue_enter()
Date
On Wed, 2018-03-14 at 11:46 -0700, tj@kernel.org wrote:
> > that are ordered with an RCU read lock (https://lwn.net/Articles/573497/). See
> > also the following comment in scsi_device_quiesce():
> >
> > /*
> > * Ensure that the effect of blk_set_preempt_only() will be visible
> > * for percpu_ref_tryget() callers that occur after the queue
> > * unfreeze even if the queue was already frozen before this function
> > * was called. See also https://lwn.net/Articles/573497/.
> > */
> >
> > Since this patch introduces a subtle and hard to debug race condition, please
> > drop this patch.
>
> Hah, the pairing is between scsi_device_quiesce() and
> blk_queue_enter()? But that doesn't make sense either because
> scsi_device_quiesce() is doing regular synchronize_rcu() and
> blk_queue_enter() is doing rcu_read_lock_sched(). They don't
> interlock with each other in any way.

Can you clarify this further? From <linux/rcupdate.h>:

static inline void synchronize_rcu(void)
{
synchronize_sched();
}

> So, the right thing to do here would be somehow moving the RCU
> synchronization into blk_set_preempt_only() and switching to regular
> RCU protection in blk_queue_enter(). The code as-is isn't really
> doing anything.

Since the RCU protection in blk_queue_enter() surrounds a percpu_ref_tryget_live()
call and since percpu_ref_tryget_live() calls rcu_read_lock/unlock_sched() itself
I don't think that it is allowed to switch to regular RCU protection in
blk_queue_enter() without modifying the RCU implementation.

Bart.


\
 
 \ /
  Last update: 2018-03-14 21:05    [W:0.076 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site