Messages in this thread | | | From | John Stultz <> | Date | Tue, 22 Aug 2023 12:24:56 -0700 | Subject | Re: [PATCH v5 02/19] locking/mutex: Removes wakeups from under mutex::wait_lock |
| |
On Tue, Aug 22, 2023 at 12:11 PM Waiman Long <longman@redhat.com> wrote: > On 8/19/23 02:08, John Stultz wrote: > > @@ -676,6 +677,10 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas > > } > > > > raw_spin_unlock(&lock->wait_lock); > > + /* Make sure we do wakeups before calling schedule */ > > + wake_up_q(&wake_q); > > + wake_q_init(&wake_q); > > + > > The wake_q may have task to wake up only in the case of ww_mutex which > is a minority in the kernel. IOW, wake_up_q() which is a function call > will do nothing in most cases. From an optimization point of view, it is > better to do a "!wake_q_empty(&wake_q)" check before calling wake_up_q().
Thanks for the suggestion! Updated for the next version!
> > @@ -946,9 +953,11 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne > > if (owner & MUTEX_FLAG_HANDOFF) > > __mutex_handoff(lock, next); > > > > + preempt_disable(); > > raw_spin_unlock(&lock->wait_lock); > > > > wake_up_q(&wake_q); > > + preempt_enable(); > > } > > I think it looks better to put the preempt_disable() right before > raw_spin_lock() for proper nesting.
Agreed.
Thanks so much for the review and feedback! I really appreciate it! -john
| |