lkml.org 
[lkml]   [2008]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RT PATCH v2] seqlock: serialize against writers
Peter Zijlstra wrote:
> On Sat, 2008-08-30 at 08:32 -0400, Gregory Haskins wrote:
>
>> Peter Zijlstra wrote:
>>
>>> On Fri, 2008-08-29 at 14:03 -0400, Gregory Haskins wrote:
>>>
>>>
>>>> *Patch submitted for inclusion in PREEMPT_RT 26-rt4. Applies to 2.6.26.3-rt3*
>>>>
>>>> Hi Ingo, Steven, Thomas,
>>>> Please consider for -rt4. This fixes a nasty deadlock on my systems under
>>>> heavy load.
>>>>
>>>> [
>>>> Changelog:
>>>> v2: only touch seqlock_t because raw_seqlock_t doesn't require
>>>> serialization and userspace cannot modify data during a read
>>>>
>>>> v1: initial release
>>>> ]
>>>>
>>>> -Greg
>>>>
>>>> ----
>>>> seqlock: serialize against writers
>>>>
>>>> Seqlocks have always advertised that readers do not "block", but this was
>>>> never really true. Readers have always logically blocked at the head of
>>>> the critical section under contention with writers, regardless of whether
>>>> they were allowed to run code or not.
>>>>
>>>> Recent changes in this space (88a411c07b6fedcfc97b8dc51ae18540bd2beda0)
>>>> have turned this into a more explicit blocking operation in mainline.
>>>> However, this change highlights a short-coming in -rt because the
>>>> normal seqlock_ts are preemptible. This means that we can potentially
>>>> deadlock should a reader spin waiting for a write critical-section to end
>>>> while the writer is preempted.
>>>>
>>>>
>>> Ah, the point I was missing is higher-priority realtime task, in which
>>> case the write side will never run because it wont preempt.
>>>
>>>
>> Yep
>>
>>>
>>>
>>>> This patch changes the internal implementation to use a rwlock and forces
>>>> the readers to serialize with the writers under contention. This will
>>>> have the advantage that -rt seqlocks_t will sleep the reader if deadlock
>>>> were imminent, and it will pi-boost the writer to prevent inversion.
>>>>
>>>> This fixes a deadlock discovered under testing where all high prioritiy
>>>> readers were hogging the cpus and preventing a writer from releasing the
>>>> lock.
>>>>
>>>> Since seqlocks are designed to be used as rarely-write locks, this should
>>>> not affect the performance in the fast-path
>>>>
>>>>
>>> Still dont like this patch, once you have a rwlock you might as well go
>>> all the way.
>>>
>> Why?
>>
>
> Because the second point.
>
>
>> A full rwlock will still be much slower since the readers will
>> always need an atomic op. This construct only uses atomic ops in the
>> slow path under contention, which should be rare, and is thus still
>> superior when retries are permissible to the design.
>>
>>
>>> Esp since this half-arsed construct defeats PI in certain
>>> cases.
>>>
>>>
>> Ouch. While I admit that you can still get into inversion scenarios
>> once the reader leaves the seqbegin, this is the nature of seqlocks.
>> The only ways I can think of to get around this involve atomic ops in
>> the fast path, which I think should be avoided. What would you suggest
>> otherwise?
>>
>
> Since we're talking -rt here, determinism rules, so bite the bullet and
> do full PI.
>
> The only reason we made all that stuff preemptable is to gain
> determinism, that also means we have to do the PI thing.
>
Yeah, you have a point. I still think this patch will solve the
deadlock thing, so please consider it in the interim. I will whip up a
full PI solution next week.

-Greg


[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-08-30 15:11    [W:0.040 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site