Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470531 > unrolled thread

Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery

Back to article view | Back to linux.kernel

Contents

  Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery Andreas Mohr <andi@lisas.de> - 2016-08-26 05:30 +0200
    Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-08-26 11:20 +0200

#1470531 — Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery

Hi,

[no properly binding reference via In-Reply-To: available thus manually re-creating, sorry]

> > But initerim I guess we could set our own owner field and check that
> > to keep the duct-tape from getting off completely.
> > -Daniel
> 
> Another alternative is to provide a standard mutex API that returns the
> owner of the lock if there is a real need for this capability. Peeking
> into lock internal is not a good practice.

From personal experience here I would suggest that
the core issue here is that
this would create an inherently race-window-tainted API,
which clearly is something to be avoided:

The point is that the lock *owner* value is *volatile*
whenever it is *not* our own context instance
that is currently holding the lock
while querying this API
(i.e., thus not guaranteeing that the owner value will *not* be changed interim!),
since in such a situation
(not-privately-locked case!!)
lock ownership may change at any point from under
our just-observed result value.

Returning such inherently racy information from a publicly offered mutex API
is, errrrr, not so good, to put it rather mildly.

So, it seems the most we could provide
which would offer a reliable, non-racy API protocol
is something like:

static bool mutex_is_locked_by_us(struct mutex *mutex)

since during execution of this processing it would be guaranteed that:
- either we do have the lock, thus *we* *RELIABLY* are and will be "the owner"
- or we simply do not have it, thus *we* *RELIABLY* are and will be "not the owner"


[but note that in that case
this mutex API implementation code would have
a potentially unholy dependency on task stuff such as "current",
but which it probably already has anyway]

HTH,

Andreas Mohr

[toc] | [next] | [standalone]

#1470609

On Fri, Aug 26, 2016 at 05:25:09AM +0200, Andreas Mohr wrote:
> > Another alternative is to provide a standard mutex API that returns the
> > owner of the lock if there is a real need for this capability. Peeking
> > into lock internal is not a good practice.


> So, it seems the most we could provide which would offer a reliable,
> non-racy API protocol is something like:
> 
> static bool mutex_is_locked_by_us(struct mutex *mutex)
> 
> since during execution of this processing it would be guaranteed that:
> - either we do have the lock, thus *we* *RELIABLY* are and will be "the owner"
> - or we simply do not have it, thus *we* *RELIABLY* are and will be "not the owner"

Right, and that is exactly what they attempted and need. And the new
mutex implementation could actually do this much better than the old
one.

But yes, such an interface should be part of the mutex implementation
proper, not something hacked on in random places.

Fwiw, the build bot seems to have found another instance of this thing
:/ drivers/gpu/drm/msm/msm_gem_shrinker.c includes an exact copy.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web