On 05/18, Ingo Molnar wrote:
>
>
> * Matthew Wilcox <[email protected]> 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.

Reply via email to