* Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > On 05/18, Ingo Molnar wrote: > > > > > > * Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > > This is confusingly written. I think you mean ... > > > > > > if (!owner) > > > goto done; > > > if (!is_rwsem_owner_spinnable(owner)) { > > > ret = false; > > > goto done; > > > } > > > > Yes, that's cleaner. Waiman, mind sending a followup patch that cleans this up? > > Or simply > > static inline bool owner_on_cpu(struct task_struct *owner) > { > return owner->on_cpu && !vcpu_is_preempted(task_cpu(owner)); > } > > static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem) > { > struct task_struct *owner; > bool ret = true; > > if (need_resched()) > return false; > > rcu_read_lock(); > owner = READ_ONCE(sem->owner); > if (owner) { > ret = is_rwsem_owner_spinnable(owner) && > owner_on_cpu(owner); > } > rcu_read_unlock(); > return ret; > } > > note that rwsem_spin_on_owner() can use the new owner_on_cpu() helper too, > > if (need_resched() || !owner_on_cpu(owner)) { > rcu_read_unlock(); > return false; > } > > looks a bit better than the current code: > > if (!owner->on_cpu || need_resched() || > vcpu_is_preempted(task_cpu(owner))) { > rcu_read_unlock(); > return false; > } > > Oleg. That looks good to me too - mind sending a patch on top of latest -tip? Thanks, Ingo