lkml.org 
[lkml]   [2013]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 00/44] ldisc patchset
From
On Mon, Mar 11, 2013 at 1:44 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> Greg,
> This patchset includes
> 'tty: Drop lock contention stat from ldsem trylocks'
> so no need to apply that on this series. Also, I noticed you
> kept the 'tty is NULL' removal on a different branch so I left
> my patch in this series that removes it.
>
> This series applies cleanly to tty-next.
>
> v5 changes:
>
> After completing an audit of the recursive use of ldisc
> references, I discovered the _blocking_ recursive acquisition
> of ldisc references was limited to line disciplines misusing
> the tty_perform_flush() function.
> With that now resolved in,
> 'tty: Fix recursive deadlock in tty_perform_flush()'
> the recursion design in ldsem has been removed.
>
> The recursion removal is in its own patch,
> 'tty: Remove ldsem recursion support'
> to ease review for those that have already reviewed the
> ldsem implementation.
>
> In addition, this patchset implements lock stealing derived
> from the work of Michel Lespinasse <walken@google.com> on
> writer lock stealing in rwsem.
>
> Although the rwsem write lock stealing changes are motivated
> by performance criteria, these changes are motivated by reduced
> code line count and simplicity of design.
>
> *** Edited below to remove recursion discussion ***
>
> Back in early December I realized that a classic read/write semaphore
> with writer priority was the ideal mechanism for handling the
> line discipline referencing.
>
> Line discipline references act as "readers"; closing or changing the
> line discipline is prevented while these references are outstanding.
> Conversely, line discipline references should not be granted while
> the line discipline is closing or changing; these are the "writers".
>
> Unfortunately, the existing rwsem uses a FIFO priority for
> waiting threads and does not support timeouts.
>
> So this implements just that: a writer-priority
> read/write semaphore with timed waits.

Thanks for eliminating the recursion requirement. I think this really
helps - I didn't like that multiple readers with a colliding current
hash could basically starve out a writer forever.

Not knowing anything about the tty layer, I am curious about the
context for your other requirements. What are ldisc references taken
for and for how long are they held ? I am surprised that the writers
may hit a 5 second timeout (because I didn't expect the references to
be held for very long).

Also why the write-priority requirement rather than reader-writer
fairness ? Is it to make it less likely to hit the writer timeouts ?

In short: I am worried about the introduciton of a new lock type, and
would be happier if rwsem could be made to fit. BTW, extending rwsem
itself to add writer timeouts seems quite doable (but making it work
as a write priority lock would seem like a bad idea).

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


\
 
 \ /
  Last update: 2013-03-12 04:01    [W:1.969 / U:2.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site