Skip to content

Commit

Permalink
locking/mutex, drm: Introduce mutex_trylock_recursive()
Browse files Browse the repository at this point in the history
By popular DRM demand, introduce mutex_trylock_recursive() to fix up the
two GEM users.

Without this it is very easy for these drivers to get stuck in
low-memory situations and trigger OOM. Work is in progress to remove the
need for this in at least i915.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Ding Tianhong <dingtianhong@huawei.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jason Low <jason.low2@hpe.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Terry Rudd <terry.rudd@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Will Deacon <Will.Deacon@arm.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Nov 15, 2016
1 parent 83f0616 commit 0f5225b
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 7 deletions.
15 changes: 12 additions & 3 deletions drivers/gpu/drm/i915/i915_gem_shrinker.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,20 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)

static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
{
if (!mutex_trylock(&dev->struct_mutex))
switch (mutex_trylock_recursive(&dev->struct_mutex)) {
case MUTEX_TRYLOCK_FAILED:
return false;

*unlock = true;
return true;
case MUTEX_TRYLOCK_SUCCESS:
*unlock = true;
return true;

case MUTEX_TRYLOCK_RECURSIVE:
*unlock = false;
return true;
}

BUG();
}

static unsigned long
Expand Down
16 changes: 12 additions & 4 deletions drivers/gpu/drm/msm/msm_gem_shrinker.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,21 @@

static bool msm_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
{
if (!mutex_trylock(&dev->struct_mutex))
switch (mutex_trylock_recursive(&dev->struct_mutex)) {
case MUTEX_TRYLOCK_FAILED:
return false;

*unlock = true;
return true;
}
case MUTEX_TRYLOCK_SUCCESS:
*unlock = true;
return true;

case MUTEX_TRYLOCK_RECURSIVE:
*unlock = false;
return true;
}

BUG();
}

static unsigned long
msm_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
Expand Down
31 changes: 31 additions & 0 deletions include/linux/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,35 @@ extern void mutex_unlock(struct mutex *lock);

extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);

/*
* These values are chosen such that FAIL and SUCCESS match the
* values of the regular mutex_trylock().
*/
enum mutex_trylock_recursive_enum {
MUTEX_TRYLOCK_FAILED = 0,
MUTEX_TRYLOCK_SUCCESS = 1,
MUTEX_TRYLOCK_RECURSIVE,
};

/**
* mutex_trylock_recursive - trylock variant that allows recursive locking
* @lock: mutex to be locked
*
* This function should not be used, _ever_. It is purely for hysterical GEM
* raisins, and once those are gone this will be removed.
*
* Returns:
* MUTEX_TRYLOCK_FAILED - trylock failed,
* MUTEX_TRYLOCK_SUCCESS - lock acquired,
* MUTEX_TRYLOCK_RECURSIVE - we already owned the lock.
*/
static inline __deprecated __must_check enum mutex_trylock_recursive_enum
mutex_trylock_recursive(struct mutex *lock)
{
if (unlikely(__mutex_owner(lock) == current))
return MUTEX_TRYLOCK_RECURSIVE;

return mutex_trylock(lock);
}

#endif /* __LINUX_MUTEX_H */
6 changes: 6 additions & 0 deletions scripts/checkpatch.pl
Original file line number Diff line number Diff line change
Expand Up @@ -6076,6 +6076,12 @@ sub process {
}
}

# check for mutex_trylock_recursive usage
if ($line =~ /mutex_trylock_recursive/) {
ERROR("LOCKING",
"recursive locking is bad, do not use this ever.\n" . $herecurr);
}

# check for lockdep_set_novalidate_class
if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ ||
$line =~ /__lockdep_no_validate__\s*\)/ ) {
Expand Down

0 comments on commit 0f5225b

Please sign in to comment.