lkml.org 
[lkml]   [2019]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 09/16] locking/rwsem: Ensure an RT task will not spin on reader
From
Date
On 04/18/2019 04:52 AM, Peter Zijlstra wrote:
> 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.

Right. That is the plan.

>
> 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...

Well, there is limitation in merging owner to rwsem. First of all, we
can't do that for 32-bit. Right now owner merging is enabled for x86-64
only. I will need to study the max physical address bits for the other
architectures later on once I am done with this patchset. So doing an
extra loop will still be helpful.

Cheers,
Longman

\
 
 \ /
  Last update: 2019-04-18 15:29    [W:0.134 / U:3.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site