* Oleg Nesterov <o...@redhat.com> wrote:

> On 05/18, Ingo Molnar wrote:
> >
> >
> > * Matthew Wilcox <wi...@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

Reply via email to