[Intel-gfx] [PATCH 60/61] drm/i915: Finally remove obj->mm.lock.

Maarten Lankhorst
2020-10-02T13:00:33Z

With all callers and selftests fixed to use ww locking, we can now
finally remove this lock.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 2 -
drivers/gpu/drm/i915/gem/i915_gem_object.h | 5 +--
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 -
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 38 ++++---------------
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 34 ++++-------------
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 37 +++++++++++++-----
drivers/gpu/drm/i915/gem/i915_gem_shrinker.h | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 2 -
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +-
drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
drivers/gpu/drm/i915/i915_gem.c | 8 +---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
13 files changed, 53 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index af5d561de7dc..afea180f500d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -62,8 +62,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
const struct drm_i915_gem_object_ops *ops,
struct lock_class_key *key, unsigned flags)
{
- mutex_init(&obj->mm.lock);
-
spin_lock_init(&obj->vma.lock);
INIT_LIST_HEAD(&obj->vma.list);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 135a7a16d2d4..6afa2018a252 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -123,7 +123,7 @@ static inline void assert_object_held_shared(struct drm_i915_gem_object *obj)
*/
if (IS_ENABLED(CONFIG_LOCKDEP) &&
kref_read(&obj->base.refcount) > 0)
- lockdep_assert_held(&obj->mm.lock);
+ assert_object_held(obj);
}

static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj,
@@ -311,7 +311,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
static inline int __must_check
i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
{
- might_lock(&obj->mm.lock);
+ assert_object_held(obj);

if (atomic_inc_not_zero(&obj->mm.pages_pin_count))
return 0;
@@ -357,7 +357,6 @@ i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
}

int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
-int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj);
void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
void i915_gem_object_writeback(struct drm_i915_gem_object *obj);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index d3f65df6d6b3..09b4df17c5d1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -201,7 +201,6 @@ struct drm_i915_gem_object {
* Protects the pages and their use. Do not use directly, but
* instead go through the pin/unpin interfaces.
*/
- struct mutex lock;
atomic_t pages_pin_count;
atomic_t shrink_pin;

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 84711dad5ca8..88c905624363 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -65,7 +65,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
struct list_head *list;
unsigned long flags;

- lockdep_assert_held(&obj->mm.lock);
+ assert_object_held(obj);
spin_lock_irqsave(&i915->mm.obj_lock, flags);

i915->mm.shrink_count++;
@@ -112,9 +112,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
{
int err;

- err = mutex_lock_interruptible(&obj->mm.lock);
- if (err)
- return err;
+ assert_object_held(obj);

assert_object_held_shared(obj);

@@ -123,15 +121,13 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)

err = ____i915_gem_object_get_pages(obj);
if (err)
- goto unlock;
+ return err;

smp_mb__before_atomic();
}
atomic_inc(&obj->mm.pages_pin_count);

-unlock:
- mutex_unlock(&obj->mm.lock);
- return err;
+ return 0;
}

int i915_gem_object_pin_pages_unlocked(struct drm_i915_gem_object *obj)
@@ -218,7 +214,7 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
return pages;
}

-int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj)
+int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
{
struct sg_table *pages;

@@ -249,21 +245,6 @@ int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj)
return 0;
}

-int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
-{
- int err;
-
- if (i915_gem_object_has_pinned_pages(obj))
- return -EBUSY;
-
- /* May be called by shrinker from within get_pages() (on another bo) */
- mutex_lock(&obj->mm.lock);
- err = __i915_gem_object_put_pages_locked(obj);
- mutex_unlock(&obj->mm.lock);
-
- return err;
-}
-
static inline pte_t iomap_pte(resource_size_t base,
dma_addr_t offset,
pgprot_t prot)
@@ -380,9 +361,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
!i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_IOMEM))
return ERR_PTR(-ENXIO);

- err = mutex_lock_interruptible(&obj->mm.lock);
- if (err)
- return ERR_PTR(err);
+ assert_object_held(obj);

pinned = !(type & I915_MAP_OVERRIDE);
type &= ~I915_MAP_OVERRIDE;
@@ -424,15 +403,12 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
obj->mm.mapping = page_pack_bits(ptr, type);
}

-out_unlock:
- mutex_unlock(&obj->mm.lock);
return ptr;

err_unpin:
atomic_dec(&obj->mm.pages_pin_count);
err_unlock:
- ptr = ERR_PTR(err);
- goto out_unlock;
+ return ERR_PTR(err);
}

void *i915_gem_object_pin_map_unlocked(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 15d8f8d52cbe..fee2e1ffba07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -184,40 +184,22 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
if (err)
return err;

- err = mutex_lock_interruptible(&obj->mm.lock);
- if (err)
- return err;
+ if (obj->mm.madv != I915_MADV_WILLNEED)
+ return -EFAULT;

- if (unlikely(!i915_gem_object_has_struct_page(obj)))
- goto out;
+ if (obj->mm.quirked)
+ return -EFAULT;

- if (obj->mm.madv != I915_MADV_WILLNEED) {
- err = -EFAULT;
- goto out;
- }
-
- if (obj->mm.quirked) {
- err = -EFAULT;
- goto out;
- }
-
- if (obj->mm.mapping || i915_gem_object_has_pinned_pages(obj)) {
- err = -EBUSY;
- goto out;
- }
+ if (obj->mm.mapping || i915_gem_object_has_pinned_pages(obj))
+ return -EBUSY;

if (unlikely(obj->mm.madv != I915_MADV_WILLNEED)) {
drm_dbg(obj->base.dev,
"Attempting to obtain a purgeable object\n");
- err = -EFAULT;
- goto out;
+ return -EFAULT;
}

- err = i915_gem_object_shmem_to_phys(obj);
-
-out:
- mutex_unlock(&obj->mm.lock);
- return err;
+ return i915_gem_object_shmem_to_phys(obj);
}

#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index e0778b3cc0c3..5ae09be61c0b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -99,7 +99,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
goto err_sg;
}

- i915_gem_shrink(i915, 2 * page_count, NULL, *s++);
+ i915_gem_shrink(NULL, i915, 2 * page_count, NULL, *s++);

/*
* We've tried hard to allocate the memory by reaping
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index afc6e5b4dcf1..e42192834c88 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -93,7 +93,8 @@ static void try_to_writeback(struct drm_i915_gem_object *obj,
* The number of pages of backing storage actually released.
*/
unsigned long
-i915_gem_shrink(struct drm_i915_private *i915,
+i915_gem_shrink(struct i915_gem_ww_ctx *ww,
+ struct drm_i915_private *i915,
unsigned long target,
unsigned long *nr_scanned,
unsigned int shrink)
@@ -112,6 +113,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
intel_wakeref_t wakeref = 0;
unsigned long count = 0;
unsigned long scanned = 0;
+ int err;

trace_i915_gem_shrink(i915, target, shrink);

@@ -199,23 +201,38 @@ i915_gem_shrink(struct drm_i915_private *i915,

spin_unlock_irqrestore(&i915->mm.obj_lock, flags);

- if (unsafe_drop_pages(obj, shrink) &&
- mutex_trylock(&obj->mm.lock)) {
+ err = 0;
+ if (unsafe_drop_pages(obj, shrink)) {
/* May arrive from get_pages on another bo */
- if (!__i915_gem_object_put_pages_locked(obj)) {
+ if (!ww) {
+ if (!i915_gem_object_trylock(obj))
+ goto skip;
+ } else {
+ err = i915_gem_object_lock(obj, ww);
+ if (err)
+ goto skip;
+ }
+
+ if (!__i915_gem_object_put_pages(obj)) {
try_to_writeback(obj, shrink);
count += obj->base.size >> PAGE_SHIFT;
}
- mutex_unlock(&obj->mm.lock);
+ if (!ww)
+ i915_gem_object_unlock(obj);
}

scanned += obj->base.size >> PAGE_SHIFT;
+skip:
i915_gem_object_put(obj);

spin_lock_irqsave(&i915->mm.obj_lock, flags);
+ if (err)
+ break;
}
list_splice_tail(&still_in_list, phase->list);
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
+ if (err)
+ return err;
}

if (shrink & I915_SHRINK_BOUND)
@@ -246,7 +263,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *i915)
unsigned long freed = 0;

with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
- freed = i915_gem_shrink(i915, -1UL, NULL,
+ freed = i915_gem_shrink(NULL, i915, -1UL, NULL,
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND);
}
@@ -292,7 +309,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)

sc->nr_scanned = 0;

- freed = i915_gem_shrink(i915,
+ freed = i915_gem_shrink(NULL, i915,
sc->nr_to_scan,
&sc->nr_scanned,
I915_SHRINK_BOUND |
@@ -301,7 +318,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
intel_wakeref_t wakeref;

with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
- freed += i915_gem_shrink(i915,
+ freed += i915_gem_shrink(NULL, i915,
sc->nr_to_scan - sc->nr_scanned,
&sc->nr_scanned,
I915_SHRINK_ACTIVE |
@@ -326,7 +343,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)

freed_pages = 0;
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
- freed_pages += i915_gem_shrink(i915, -1UL, NULL,
+ freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND |
I915_SHRINK_WRITEBACK);
@@ -364,7 +381,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
intel_wakeref_t wakeref;

with_intel_runtime_pm(&i915->runtime_pm, wakeref)
- freed_pages += i915_gem_shrink(i915, -1UL, NULL,
+ freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND |
I915_SHRINK_VMAPS);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.h b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.h
index b397d7785789..8512470f6fd6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.h
@@ -9,10 +9,12 @@
#include <linux/bits.h>

struct drm_i915_private;
+struct i915_gem_ww_ctx;
struct mutex;

/* i915_gem_shrinker.c */
-unsigned long i915_gem_shrink(struct drm_i915_private *i915,
+unsigned long i915_gem_shrink(struct i915_gem_ww_ctx *ww,
+ struct drm_i915_private *i915,
unsigned long target,
unsigned long *nr_scanned,
unsigned flags);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index ffcaee74a249..4523a14db86e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -265,7 +265,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
* pages to prevent them being swapped out and causing corruption
* due to the change in swizzling.
*/
- mutex_lock(&obj->mm.lock);
if (i915_gem_object_has_pages(obj) &&
obj->mm.madv == I915_MADV_WILLNEED &&
i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
@@ -280,7 +279,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
obj->mm.quirked = true;
}
}
- mutex_unlock(&obj->mm.lock);

spin_lock(&obj->vma.lock);
for_each_ggtt_vma(vma, obj) {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 55aa6fc3b518..327f01b04f21 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -235,7 +235,7 @@ static int i915_gem_object_userptr_unbind(struct drm_i915_gem_object *obj, bool
if (GEM_WARN_ON(i915_gem_object_has_pinned_pages(obj)))
return -EBUSY;

- mutex_lock(&obj->mm.lock);
+ assert_object_held(obj);

pages = __i915_gem_object_unset_pages(obj);
if (!IS_ERR_OR_NULL(pages))
@@ -243,7 +243,6 @@ static int i915_gem_object_userptr_unbind(struct drm_i915_gem_object *obj, bool

if (get_pages)
err = ____i915_gem_object_get_pages(obj);
- mutex_unlock(&obj->mm.lock);

return err;
}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ea469168cd44..c5c7f77ee8dd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1508,10 +1508,10 @@ i915_drop_caches_set(void *data, u64 val)

fs_reclaim_acquire(GFP_KERNEL);
if (val & DROP_BOUND)
- i915_gem_shrink(i915, LONG_MAX, NULL, I915_SHRINK_BOUND);
+ i915_gem_shrink(NULL, i915, LONG_MAX, NULL, I915_SHRINK_BOUND);

if (val & DROP_UNBOUND)
- i915_gem_shrink(i915, LONG_MAX, NULL, I915_SHRINK_UNBOUND);
+ i915_gem_shrink(NULL, i915, LONG_MAX, NULL, I915_SHRINK_UNBOUND);

if (val & DROP_SHRINK_ALL)
i915_gem_shrink_all(i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4b5afb85efd1..7e4fa1cad82c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1103,10 +1103,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
if (err)
goto out;

- err = mutex_lock_interruptible(&obj->mm.lock);
- if (err)
- goto out_ww;
-
if (i915_gem_object_has_pages(obj) &&
i915_gem_object_is_tiled(obj) &&
i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
@@ -1149,9 +1145,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
i915_gem_object_truncate(obj);

args->retained = obj->mm.madv != __I915_MADV_PURGED;
- mutex_unlock(&obj->mm.lock);

-out_ww:
i915_gem_object_unlock(obj);
out:
i915_gem_object_put(obj);
@@ -1335,7 +1329,7 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)

wakeref = intel_runtime_pm_get(&i915->runtime_pm);

- i915_gem_shrink(i915, -1UL, NULL, ~0);
+ i915_gem_shrink(NULL, i915, -1UL, NULL, ~0);
i915_gem_drain_freed_objects(i915);

list_for_each_entry(obj, &i915->mm.shrink_list, mm.link) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c5ee1567f3d1..729074ee33d4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -44,7 +44,7 @@ int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj,
* the DMA remapper, i915_gem_shrink will return 0.
*/
GEM_BUG_ON(obj->mm.pages == pages);
- } while (i915_gem_shrink(to_i915(obj->base.dev),
+ } while (i915_gem_shrink(NULL, to_i915(obj->base.dev),
obj->base.size >> PAGE_SHIFT, NULL,
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND));