[PATCH] drm/amd: clarify amdgpu_mn_read_lock !blocking mode
From: Michal Hocko
Date: Fri Aug 24 2018 - 09:45:52 EST
Tetsuo has noticed that 93065ac753e4 ("mm, oom: distinguish blockable
mode for mmu notifiers") !blocking case for amdgpu_mn_read_lock is
incomplete because it might sleep on the notifier lock. This is true
but as it turned out from the discussion with Christian this doesn't
really matter.
The amd notifier lock doesn't block in the exclusive mode. It only ever
sleeps with the read lock inside amdgpu_mn_invalidate_node. That one
is not called in !blockable state so while we might sleep on notifier
read_lock this will only be for a short while. The same applies on the
notifier lock.
Therefore remove blockable handling from amdgpu_mn_read_lock and
document it properly.
Noticed-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: Christian Kö <christian.koenig@xxxxxxx>
Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..48fa152231be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(&amn->read_lock);
- else if (!mutex_trylock(&amn->read_lock))
- return -EAGAIN;
-
+ /*
+ * We can take sleepable lock even on !blockable mode because
+ * read_lock is only ever take from this path and the notifier
+ * lock never really sleeps. In fact the only reason why the
+ * later is sleepable is because the notifier itself might sleep
+ * in amdgpu_mn_invalidate_node but blockable mode is handled
+ * before calling into that path.
+ */
+ mutex_lock(&amn->read_lock);
if (atomic_inc_return(&amn->recursion) == 1)
down_read_non_owner(&amn->lock);
mutex_unlock(&amn->read_lock);
--
2.18.0
--
Michal Hocko
SUSE Labs