Re: initialize a mutex into locked state?

[Oleg Drokin]

On Jun 17, 2016, at 10:42 AM, Jeff Layton wrote:

> On Fri, 2016-06-17 at 10:40 -0400, Oleg Drokin wrote:
>> On Jun 17, 2016, at 10:32 AM, Peter Zijlstra wrote:
>> 
>>> 
>>> On Fri, Jun 17, 2016 at 10:24:32AM -0400, Oleg Drokin wrote:
>>>> 
>>>> 
>>>> On Jun 17, 2016, at 10:19 AM, Peter Zijlstra wrote:
>>>> 
>>>>> 
>>>>> On Fri, Jun 17, 2016 at 10:14:10AM -0400, Oleg Drokin wrote:
>>>>>> 
>>>>>> 
>>>>>> On Jun 17, 2016, at 4:25 AM, Peter Zijlstra wrote:
>>>>>> 
>>>>>>> 
>>>>>>> On Wed, Jun 15, 2016 at 02:23:35PM -0400, Oleg Drokin
>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> Hello!
>>>>>>>> 
>>>>>>>> To my surprise I found out that it's not possible to
>>>>>>>> initialise a mutex into
>>>>>>>> a locked state.
>>>>>>>> I discussed it with Arjan and apparently there's no
>>>>>>>> fundamental reason
>>>>>>>> not to allow this.
>>>>>>> There is. A mutex _must_ have an owner. If you can
>>>>>>> initialize it in
>>>>>>> locked state, you could do so statically, ie. outside of
>>>>>>> the context of
>>>>>>> a task.
>>>>>> What's wrong with disallowing only static initializers, but
>>>>>> allowing dynamic ones?
>>>>>> Then there is a clear owner.
>>>>> At which point, what wrong with the simple:
>>>>> 
>>>>> 	mutex_init(&m);
>>>>> 	mutex_lock(&m);
>>>>> 
>>>>> Sequence? Its obvious, has clear semantics and doesn't extend
>>>>> the API.
>>>> The problem is:
>>>> 
>>>> spin_lock(somelock);
>>>> structure = some_internal_list_lookup(list);
>>>> if (structure)
>>>> 	goto out;
>>>> 
>>>> init_new_structure(new_structure);
>>>> mutex_init(&new_structure->s_mutex);
>>>> mutex_lock(&new_structure->s_mutex);  // XXX CANNOT DO THIS UNDER
>>>> SPINLOCK!
>>> 	mutex_trylock(&new_structure->s_mutex);
>>> 
>>> should work, since you know it cannot be acquired yet by anybody
>>> else,
>>> since you've not published it yet.
>> This does work, but suddenly does not look so obvious anymore, does
>> it?
>> I got some feedback that doing this is not really preferred.
>> 
>> Also once __must_check is added to mutex_try_lock() (surprised it's
>> not yet),
>> we'll need to also have the useless "but what if it did fail to lock"
>> path?
>> 
> 
> Maybe just BUG in that case, and add a comment that says something
> along the lines of "this should always work since it's not hashed yet"
> ?

Yes, we can add all sorts of checks that have various impacts on code readability,
we can also move code around that also have code readability and CPU impact.

But in my discussion with Arjan he said this is a new use case that was not met before
and suggested to mail it to the list.

>>> And a trylock does not sleep, so is perfectly fine under a
>>> spinlock.
>>> 
>>>> 
>>>> 
>>>> list_add(list, new_structure->s_list);
>>>> structure = new_structure;
>>>> out:
>>>> spin_unlock(somelock);
>>>> return structure;
>>>> 
> 
> -- 
> Jeff Layton <jlayton@xxxxxxxxxxxxxxx>