[PATCH v2 1/2] drm/i915: convert i915_active.count from atomic_t to refcount_t

From: Deepak R Varma
Date: Sun Dec 25 2022 - 02:48:22 EST


The refcount_* APIs are designed to address known issues with the
atomic_t APIs for reference counting. They provide following distinct
advantages:
- protect the reference counters from overflow/underflow
- avoid use-after-free errors
- provide improved memory ordering guarantee schemes
- neater and safer.
Hence, convert the atomic_t count member variable and associated
atomic_*() API calls to equivalent refcount_t type and refcount_*() API
calls.

This patch proposal address the following warnings generated by
the atomic_as_refcounter.cocci coccinelle script
atomic_add_unless

Signed-off-by: Deepak R Varma <drv@xxxxxxxxx>
---
Please note:
1. Proposed changes are compile tested only.
2. This patch 1/2 is required to be applied before patch 2/2 due to
interdependency.

Changes in v2:
1. Patch added to the patch series.
2. Handle build issues Reported-by: kernel test robot <lkp@xxxxxxxxx>
Earlier a standalone patch was sent for the i915 base driver only. The
Kernel Test Robot reported build failure for additional atomic_*() calls
specific to i915 debugging support when enabled. This version now includes
those changes as well.


drivers/gpu/drm/i915/i915_active.c | 28 +++++++++++++-----------
drivers/gpu/drm/i915/i915_active.h | 6 ++---
drivers/gpu/drm/i915/i915_active_types.h | 4 ++--
3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 7412abf166a8..5e58d8b1e947 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -92,14 +92,14 @@ static void debug_active_init(struct i915_active *ref)
static void debug_active_activate(struct i915_active *ref)
{
lockdep_assert_held(&ref->tree_lock);
- if (!atomic_read(&ref->count)) /* before the first inc */
+ if (!refcount_read(&ref->count)) /* before the first inc */
debug_object_activate(ref, &active_debug_desc);
}

static void debug_active_deactivate(struct i915_active *ref)
{
lockdep_assert_held(&ref->tree_lock);
- if (!atomic_read(&ref->count)) /* after the last dec */
+ if (!refcount_read(&ref->count)) /* after the last dec */
debug_object_deactivate(ref, &active_debug_desc);
}

@@ -133,7 +133,7 @@ __active_retire(struct i915_active *ref)
GEM_BUG_ON(i915_active_is_idle(ref));

/* return the unused nodes to our slabcache -- flushing the allocator */
- if (!atomic_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, flags))
+ if (!refcount_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, &flags))
return;

GEM_BUG_ON(rcu_access_pointer(ref->excl.fence));
@@ -179,8 +179,8 @@ active_work(struct work_struct *wrk)
{
struct i915_active *ref = container_of(wrk, typeof(*ref), work);

- GEM_BUG_ON(!atomic_read(&ref->count));
- if (atomic_add_unless(&ref->count, -1, 1))
+ GEM_BUG_ON(!refcount_read(&ref->count));
+ if (refcount_dec_not_one(&ref->count))
return;

__active_retire(ref);
@@ -189,8 +189,8 @@ active_work(struct work_struct *wrk)
static void
active_retire(struct i915_active *ref)
{
- GEM_BUG_ON(!atomic_read(&ref->count));
- if (atomic_add_unless(&ref->count, -1, 1))
+ GEM_BUG_ON(!refcount_read(&ref->count));
+ if (refcount_dec_not_one(&ref->count))
return;

if (ref->flags & I915_ACTIVE_RETIRE_SLEEPS) {
@@ -354,7 +354,7 @@ void __i915_active_init(struct i915_active *ref,
ref->cache = NULL;

init_llist_head(&ref->preallocated_barriers);
- atomic_set(&ref->count, 0);
+ refcount_set(&ref->count, 0);
__mutex_init(&ref->mutex, "i915_active", mkey);
__i915_active_fence_init(&ref->excl, NULL, excl_retire);
INIT_WORK(&ref->work, active_work);
@@ -445,7 +445,7 @@ int i915_active_add_request(struct i915_active *ref, struct i915_request *rq)

if (replace_barrier(ref, active)) {
RCU_INIT_POINTER(active->fence, NULL);
- atomic_dec(&ref->count);
+ refcount_dec(&ref->count);
}
if (!__i915_active_fence_set(active, fence))
__i915_active_acquire(ref);
@@ -488,14 +488,16 @@ i915_active_set_exclusive(struct i915_active *ref, struct dma_fence *f)
bool i915_active_acquire_if_busy(struct i915_active *ref)
{
debug_active_assert(ref);
- return atomic_add_unless(&ref->count, 1, 0);
+ return refcount_add_not_zero(1, &ref->count);
}

static void __i915_active_activate(struct i915_active *ref)
{
spin_lock_irq(&ref->tree_lock); /* __active_retire() */
- if (!atomic_fetch_inc(&ref->count))
+ if (!refcount_inc_not_zero(&ref->count)) {
+ refcount_inc(&ref->count);
debug_active_activate(ref);
+ }
spin_unlock_irq(&ref->tree_lock);
}

@@ -757,7 +759,7 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence,
void i915_active_fini(struct i915_active *ref)
{
debug_active_fini(ref);
- GEM_BUG_ON(atomic_read(&ref->count));
+ GEM_BUG_ON(refcount_read(&ref->count));
GEM_BUG_ON(work_pending(&ref->work));
mutex_destroy(&ref->mutex);

@@ -927,7 +929,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,

first = first->next;

- atomic_dec(&ref->count);
+ refcount_dec(&ref->count);
intel_engine_pm_put(barrier_to_engine(node));

kmem_cache_free(slab_cache, node);
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index 7eb44132183a..116c7c28466a 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -193,14 +193,14 @@ void i915_active_release(struct i915_active *ref);

static inline void __i915_active_acquire(struct i915_active *ref)
{
- GEM_BUG_ON(!atomic_read(&ref->count));
- atomic_inc(&ref->count);
+ GEM_BUG_ON(!refcount_read(&ref->count));
+ refcount_inc(&ref->count);
}

static inline bool
i915_active_is_idle(const struct i915_active *ref)
{
- return !atomic_read(&ref->count);
+ return !refcount_read(&ref->count);
}

void i915_active_fini(struct i915_active *ref);
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index b02a78ac87db..152a3a25d9f7 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -7,7 +7,7 @@
#ifndef _I915_ACTIVE_TYPES_H_
#define _I915_ACTIVE_TYPES_H_

-#include <linux/atomic.h>
+#include <linux/refcount.h>
#include <linux/dma-fence.h>
#include <linux/llist.h>
#include <linux/mutex.h>
@@ -23,7 +23,7 @@ struct i915_active_fence {
struct active_node;

struct i915_active {
- atomic_t count;
+ refcount_t count;
struct mutex mutex;

spinlock_t tree_lock;
--
2.34.1