Messages in this thread | | | From | Joel Fernandes <> | Date | Fri, 3 Mar 2023 14:25:23 -0500 | Subject | Re: [PATCH] pstore: Revert pmsg_lock back to a normal mutex |
| |
On Fri, Mar 3, 2023 at 1:37 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 3 Mar 2023 18:11:34 +0000 > Joel Fernandes <joel@joelfernandes.org> wrote: > > > In the normal mutex's adaptive spinning, there is no check for if there is a > > change in waiter AFAICS (ignoring ww mutex stuff for a second). > > > > I can see one may want to do that waiter-check, as spinning > > indefinitely if the lock owner is on the CPU for too long may result in > > excessing power burn. But normal mutex does not seem to do that. > > > > What makes the rtmutex spin logic different from normal mutex in this > > scenario, so that rtmutex wants to do that but normal ones dont? > > Well, the point of the patch is that I don't think they should be different > ;-)
But there's no "waiter change" thing for mutex_spin_on_owner right.
Then, should mutex_spin_on_owner() also add a call to __mutex_waiter_is_first() ?
> > Another thought is, I am wondering if all of them spinning indefinitely might > > be Ok for rtmutex as well, since as you mentioned, preemption is enabled. So > > adding the if (top_waiter != last_waiter) {...} might be unnecessary? In fact > > may be even harmful as you are disabling interrupts in the process. > > The last patch only does the interrupt disabling if the top waiter changes. > Which in practice is seldom. > > But, I don't think a non top waiter should spin if the top waiter is not > running. The top waiter is the one that will get the lock next. If the > owner releases the lock and gives it to the top waiter, then it has to go > through the wake up of the top waiter.
Correct me if I'm wrong, but I don't think it will go through schedule() after spinning, which is what adds the overhead for John.
> I don't see why a task should spin > to save a wake up if a wake up has to happen anyway.
What about regular mutexes, happens there too or no?
> > Either way, I think a comment should go on top of the "if (top_waiter != > > waiter)" check IMO. > > What type of comment?
Comment explaining why "if (top_waiter != waiter)" is essential :-).
thanks,
-Joel
| |