On Fri, 2018-07-27 at 09:57 +0800, jianchao.wang wrote:
> Something like:
>
> percpu_ref_switch_to_atomic_sync(&q->q_usage_counter);
> if (!percpu_ref_is_zero(&q->q_usage_counter)) {
> ret = -EBUSY;
> pm_runtime_mark_last_busy(q->dev);
> } else {
> blk_set_preempt_only(q);
> if (!percpu_ref_is_zero(&q->q_usage_counter) {
> ret = -EBUSY;
> pm_runtime_mark_last_busy(q->dev);
> blk_clear_preempt_only(q);
> } else {
> q->rpm_status = RPM_SUSPENDIN;
> }
> }
I think this code is racy. Because there is no memory barrier in
blk_queue_enter() between the percpu_ref_tryget_live() and the
blk_queue_preempt_only() calls, the context that sets the PREEMPT_ONLY flag
has to use synchronize_rcu() or call_rcu() to ensure that blk_queue_enter()
sees the PREEMPT_ONLY flag after it has called percpu_ref_tryget_live().
See also http://lwn.net/Articles/573497/.
Bart.
