Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller

From: Peter Rosin
Date: Fri Apr 21 2017 - 19:28:46 EST


On 2017-04-20 23:53, Peter Rosin wrote:
> On 2017-04-18 23:53, Peter Rosin wrote:
>> On 2017-04-18 13:44, Greg Kroah-Hartman wrote:
>>> On Tue, Apr 18, 2017 at 12:59:50PM +0200, Peter Rosin wrote:
>>>> On 2017-04-18 10:51, Greg Kroah-Hartman wrote:
>>>>> On Thu, Apr 13, 2017 at 06:43:07PM +0200, Peter Rosin wrote:
>
> *snip*
>
>>>>>> + if (mux->idle_state != MUX_IDLE_AS_IS &&
>>>>>> + mux->idle_state != mux->cached_state)
>>>>>> + ret = mux_control_set(mux, mux->idle_state);
>>>>>> +
>>>>>> + up_read(&mux->lock);
>>>>>
>>>>> You require a lock to be held for a "global" function? Without
>>>>> documentation? Or even a sparse marking? That's asking for trouble...
>>>>
>>>> Documentation I can handle, but where should I look to understand how I
>>>> should add sparse markings?
>>>
>>> Run sparse on the code and see what it says :)
>>
>> Will do.
>
> I just did, and even went through the trouble of getting the bleeding
> edge sparse from the git repo when sparse 0.5.0 came up empty, but it's
> all silence for me. So, how do I add sparse markings?

I looked some more into this, and the markings I find that seem related
are __acquire() and __release(). But neither mutex_lock() nor up_read()
has markings like that, so adding them when using those kinds of locks
in an imbalanced way seems like a sure way of *getting* sparse messages
about context imbalance...

So, either that, or you are talking about __must_check markings?

I feel like I'm missing something, please advise further.

Cheers,
peda