Messages in this thread | | | Date | Fri, 18 May 2018 11:40:52 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v4 1/2] locking/rwsem: Add a new RWSEM_ANONYMOUSLY_OWNED flag |
| |
* Oleg Nesterov <oleg@redhat.com> wrote:
> On 05/18, Ingo Molnar wrote: > > > > > > * Matthew Wilcox <willy@infradead.org> 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
| |