linux-kernel - Re: [PATCH v14 00/11] mux controller abstraction and iio/i2c muxes

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]

Message-ID: <f74d5619-8614-d003-3c70-710aee0b550b@axentia.se>
Date:   Tue, 25 Apr 2017 16:16:39 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Philipp Zabel <p.zabel@...gutronix.de>
CC:     Jonathan Cameron <jic23@...nel.org>,
        <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Jonathan Corbet <corbet@....net>, <linux-i2c@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-iio@...r.kernel.org>,
        <linux-doc@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Colin Ian King <colin.king@...onical.com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        <kernel@...gutronix.de>
Subject: Re: [PATCH v14 00/11] mux controller abstraction and iio/i2c muxes

On 2017-04-24 16:59, Philipp Zabel wrote:
> On Mon, 2017-04-24 at 16:36 +0200, Peter Rosin wrote:
> [...]
>>> How about an atomic use_count on the mux_control, a bool shared that is
>>> only set by the first consumer, and controls whether selecting locks?
>>
>> That has the drawback that it is hard to restore the mux-control in a safe
>> way so that exclusive consumers are allowed after the last shared consumer
>> puts the mux away.
> 
> True.
> 
>> Agreed, it's a corner case, but I had this very similar
>> patch going through the compiler when I got this mail. Does it work as well
>> as what you suggested?
> 
> Yes, this patch works just as well.

Right, as expected :-) However, I don't like it much. It divides the mux
consumers into two camps in a way that makes it difficult to select which
camp a consumer should be in.

E.g. consider the iio-mux. The current implementation only supports quick
accesses that fit the mux_control_get_shared case. But if that mux in the
future needs to grow continuous buffered accesses, I think there will be
pressure to switch it over to the exclusive mode. Because that is a lot
closer to what you are doing with the video-mux. And then what? It will be
impossible to predict if the end user is going to use buffered accesses or
not...

So, I think the best approach is to skip the distinction between shared
and exclusive consumers and instead implement the locking with an ordinary
semaphore (instead of the old rwsem or the current mutex). Semaphores don't
have the property that the same task should down/up them (mutexes require
that for lock/unlock, and is also the reason for the lockdep complaint) and
thus fits better for long-time use such as yours or the above iio-mux with
buffered accesses. It should also hopefully be cheaper that an rwsem, and
not have any downgrade_write calls thus possibly keeping Greg sufficiently
happy...

Sure, consumers can still dig themselves into a hole by not calling deselect
as they should, but at least I think it can be made to work w/o dividing the
consumers...

Cheers,
peda