On Wed, Apr 17, 2019 at 02:47:07PM -0400, Waiman Long wrote:
> >> @@ -566,13 +573,28 @@ static bool rwsem_optimistic_spin(struct
> >> rw_semaphore *sem)
> >> }
> >>
> >> /*
> >> - * When there's no owner, we might have preempted between the
> >> - * owner acquiring the lock and setting the owner field. If
> >> - * we're an RT task that will live-lock because we won't let
> >> - * the owner complete.
> >> + * An RT task cannot do optimistic spinning if it cannot
> >> + * be sure the lock holder is running or live-lock may
> >> + * happen if the current task and the lock holder happen
> >> + * to run in the same CPU.
> >> + *
> >> + * When there's no owner or is reader-owned, an RT task
> >> + * will stop spinning if the owner state is not a writer
> >> + * at the previous iteration of the loop. This allows the
> >> + * RT task to recheck if the task that steals the lock is
> >> + * a spinnable writer. If so, it can keeps on spinning.
> >> + *
> >> + * If the owner is a writer, the need_resched() check is
> >> + * done inside rwsem_spin_on_owner(). If the owner is not
> >> + * a writer, need_resched() check needs to be done here.
> >> */
> >> - if (!sem->owner && (need_resched() || rt_task(current)))
> >> - break;
> >> + if (owner_state != OWNER_WRITER) {
> >> + if (need_resched())
> >> + break;
> >> + if (is_rt_task && (prev_owner_state != OWNER_WRITER))
> >> + break;
> >> + }
> >> + prev_owner_state = owner_state;
> >>
> >> /*
> >> * The cpu_relax() call is a compiler barrier which forces
> > This patch confuses me mightily. I mean, I see what it does, but I can't
> > figure out why. The Changelog is just one big source of confusion.
>
> Sorry for confusing you. If count and owner are separate, there is a
> time lag where the owner is NULL, but the lock is not free yet.
Right.
> Similarly, the lock could be free but another task may have stolen the
> lock if the waiter bit isn't set.
> In the former case,
(free)
> an extra iteration gives it more time for the lock holder to release
> the lock.
> In the latter case,
(stolen)
> if the new lock owner is a writer and set owner in time,
> the RT task can keep on spinning. Will clarify that in the commit log
> and the comment.
Blergh.. so by going around once extra, you hope ->owner will be set
again and we keep spinning. And this is actually measurable.
Yuck yuck yuck. I much prefer getting rid of that hole, as you do later
on in the series, that would avoid this complecity. Let me continue
reading...
