linux-kernel - Re: [PATCH v5 00/44] ldisc patchset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 Mar 2013 04:36:56 -0700
From:	Michel Lespinasse <walken@...gle.com>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	Sasha Levin <levinsasha928@...il.com>,
	Dave Jones <davej@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Shawn Guo <shawn.guo@...aro.org>, linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v5 00/44] ldisc patchset

On Tue, Mar 12, 2013 at 9:47 AM, Peter Hurley <peter@...leysoftware.com> wrote:
> On Mon, 2013-03-11 at 19:28 -0700, Michel Lespinasse wrote:
>> Also why the write-priority requirement rather than reader-writer
>> fairness ? Is it to make it less likely to hit the writer timeouts ?
>
> Since tty i/o can be really [painfully] slow, allowing waiting future
> references to succeed is not an option.

All right, that makes sense after your explanation.

> I understand the concern regarding the potential proliferation of new
> lock types. Lock implementations are hard to get right, and no one wants
> to debug 7 different lock policy implementations of a read/write
> semaphore.
>
> OTOH, a lack of existing options has spawned a DIY approach without
> higher-order locks that is rarely correct, but which goes largely
> unnoticed exactly because it's not a new lock. A brief review of the
> hangs, races, and deadlocks fixed by this patchset should be convincing
> enough of that fact. In my opinion, this is the overriding concern.

Agree that having a suitable lock for your usage is much nicer than
having ad-hoc solutions.

> The two main problems with a one-size-fits-all lock policy is that,
> 1) lock experts can't realistically foresee the consequences of policy
> changes without already being experts in the subsystems in which that
> lock is used. Even domain experts may miss potential consequences, and
> 2) domain experts typically wouldn't even consider writing a new lock.
> So they make do with atomic bit states, spinlocks, reference counts,
> mutexes, and waitqueues, making a mostly-functional, higher-order lock.

Have you considered building your ldlock based on lib/rwsem-spinlock.c
instead ? i.e. having an internal spinlock to protect the ldisc
reference count and the reader and writer queues. This would seem much
simpler get right. The downside would be that a spinlock would be
taken for a short time whenever an ldisc reference is taken or
released. I don't expect that the internal spinlock would get
significant contention ?

> Perhaps a future direction for rwsem would be to provide a selectable
> lock policy (fifo, mostly-fair, writer-first) on initialization so that
> the different use cases can be easily accomodated?

Probably makes more sense to have different locks for the different
usage models IMO...

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/