Skip to content

Commit

Permalink
drm/i915: Finally remove obj->mm.lock.
Browse files Browse the repository at this point in the history
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>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210323155059.628690-62-maarten.lankhorst@linux.intel.com
  • Loading branch information
Maarten Lankhorst authored and Daniel Vetter committed Mar 24, 2021
1 parent 480ae79 commit cf41a8f
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 92 deletions.
2 changes: 0 additions & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 2 additions & 3 deletions drivers/gpu/drm/i915/gem/i915_gem_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,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,
Expand Down Expand Up @@ -358,7 +358,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;
Expand Down Expand Up @@ -404,7 +404,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);

Expand Down
1 change: 0 additions & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_object_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,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;

Expand Down
43 changes: 9 additions & 34 deletions drivers/gpu/drm/i915/gem/i915_gem_pages.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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++;
Expand Down Expand Up @@ -117,9 +117,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);

Expand All @@ -128,15 +126,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)
Expand Down Expand Up @@ -223,7 +219,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;

Expand Down Expand Up @@ -254,21 +250,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;
}

/* The 'mapping' part of i915_gem_object_pin_map() below */
static void *i915_gem_object_map_page(struct drm_i915_gem_object *obj,
enum i915_map_type type)
Expand Down Expand Up @@ -371,9 +352,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;
Expand All @@ -383,10 +362,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));

err = ____i915_gem_object_get_pages(obj);
if (err) {
ptr = ERR_PTR(err);
goto out_unlock;
}
if (err)
return ERR_PTR(err);

smp_mb__before_atomic();
}
Expand Down Expand Up @@ -421,13 +398,11 @@ 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);
goto out_unlock;
return ptr;
}

void *i915_gem_object_pin_map_unlocked(struct drm_i915_gem_object *obj,
Expand Down
34 changes: 8 additions & 26 deletions drivers/gpu/drm/i915/gem/i915_gem_phys.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,40 +234,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 (unlikely(!i915_gem_object_has_struct_page(obj)))
goto out;

if (obj->mm.madv != I915_MADV_WILLNEED) {
err = -EFAULT;
goto out;
}
if (obj->mm.madv != I915_MADV_WILLNEED)
return -EFAULT;

if (i915_gem_object_has_tiling_quirk(obj)) {
err = -EFAULT;
goto out;
}
if (i915_gem_object_has_tiling_quirk(obj))
return -EFAULT;

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)
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_pm.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)
*/

with_intel_runtime_pm(&i915->runtime_pm, wakeref)
i915_gem_shrink(i915, -1UL, NULL, ~0);
i915_gem_shrink(NULL, i915, -1UL, NULL, ~0);
i915_gem_drain_freed_objects(i915);

wbinvd_on_all_cpus();
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_shmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 27 additions & 10 deletions drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,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)
Expand All @@ -113,6 +114,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);

Expand Down Expand Up @@ -200,25 +202,40 @@ 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);
}

dma_resv_prune(obj->base.resv);

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)
Expand Down Expand Up @@ -249,7 +266,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);
}
Expand Down Expand Up @@ -295,7 +312,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 |
Expand All @@ -304,7 +321,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 |
Expand All @@ -329,7 +346,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);
Expand Down Expand Up @@ -367,7 +384,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);
Expand Down
4 changes: 3 additions & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_shrinker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_tiling.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -280,7 +279,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
i915_gem_object_set_tiling_quirk(obj);
}
}
mutex_unlock(&obj->mm.lock);

spin_lock(&obj->vma.lock);
for_each_ggtt_vma(vma, obj) {
Expand Down
3 changes: 1 addition & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_userptr.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,14 @@ 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))
i915_gem_userptr_put_pages(obj, pages);

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

return err;
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/i915_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -904,10 +904,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);
Expand Down
Loading

0 comments on commit cf41a8f

Please sign in to comment.