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" ? > > > > 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 <[email protected]>
