Hi everyone,
dma_fences have ever lived under the tyranny dictated by the module lifetime of their issuer, leading to crashes should anybody still holding a reference to a dma_fence when the module of the issuer was unloaded.
The basic problem is that when buffer are shared between drivers dma_fence objects can leak into external drivers and stay there even after they are signaled. The dma_resv object for example only lazy releases dma_fences.
So what happens is that when the module who originally created the dma_fence unloads the dma_fence_ops function table becomes unavailable as well and so any attempt to release the fence crashes the system.
Previously various approaches have been discussed, including changing the locking semantics of the dma_fence callbacks (by me) as well as using the drm scheduler as intermediate layer (by Sima) to disconnect dma_fences from their actual users, but none of them are actually solving all problems.
Tvrtko did some really nice prerequisite work by protecting the returned strings of the dma_fence_ops by RCU. This way dma_fence creators where able to just wait for an RCU grace period after fence signaling before they could be save to free those data structures.
Now this patch set here goes a step further and protects the whole dma_fence_ops structure by RCU, so that after the fence signals the pointer to the dma_fence_ops is set to NULL when there is no wait nor release callback given. All functionality which use the dma_fence_ops reference are put inside an RCU critical section, except for the deprecated issuer specific wait and of course the optional release callback.
Additional to the RCU changes the lock protecting the dma_fence state previously had to be allocated external. This set here now changes the functionality to make that external lock optional and allows dma_fences to use an inline lock and be self contained.
v4:
Rebases the whole set on upstream changes, especially the cleanup from Philip in patch "drm/amdgpu: independence for the amdkfd_fence!".
Adding two patches which brings the DMA-fence self tests up to date. The first selftest changes removes the mock_wait and so actually starts testing the default behavior instead of some hacky implementation in the test. This one got upstreamed independent of this set. The second drops the mock_fence as well and tests the new RCU and inline spinlock functionality.
v5:
Rebase on top of drm-misc-next instead of drm-tip, leave out all driver changes for now since those should go through the driver specific paths anyway.
Address a few more review comments, especially some rebase mess and typos. And finally fix one more bug found by AMDs CI system.
Especially the first patch still needs a Reviewed-by, apart from that I think I've addressed all review comments and problems.
Please review and comment, Christian.
At first glance it is counter intuitive to protect a constant function pointer table by RCU, but this allows modules providing the function table to unload by waiting for an RCU grace period.
v2: make one the now duplicated lockdep warnings a comment instead. v3: Add more documentation to ->wait and ->release callback. v4: fix typo in documentation v5: rebased on drm-tip
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/dma-buf/dma-fence.c | 69 +++++++++++++++++++++++++------------ include/linux/dma-fence.h | 29 ++++++++++++++-- 2 files changed, 73 insertions(+), 25 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 21c5c30b4f34..e3e74f98f58d 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -522,6 +522,7 @@ EXPORT_SYMBOL(dma_fence_signal); signed long dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) { + const struct dma_fence_ops *ops; signed long ret;
if (WARN_ON(timeout < 0)) @@ -533,15 +534,21 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
dma_fence_enable_sw_signaling(fence);
- if (trace_dma_fence_wait_start_enabled()) { - rcu_read_lock(); - trace_dma_fence_wait_start(fence); + rcu_read_lock(); + ops = rcu_dereference(fence->ops); + trace_dma_fence_wait_start(fence); + if (ops->wait) { + /* + * Implementing the wait ops is deprecated and not supported for + * issuer independent fences, so it is ok to use the ops outside + * the RCU protected section. + */ + rcu_read_unlock(); + ret = ops->wait(fence, intr, timeout); + } else { rcu_read_unlock(); - } - if (fence->ops->wait) - ret = fence->ops->wait(fence, intr, timeout); - else ret = dma_fence_default_wait(fence, intr, timeout); + } if (trace_dma_fence_wait_end_enabled()) { rcu_read_lock(); trace_dma_fence_wait_end(fence); @@ -562,6 +569,7 @@ void dma_fence_release(struct kref *kref) { struct dma_fence *fence = container_of(kref, struct dma_fence, refcount); + const struct dma_fence_ops *ops;
rcu_read_lock(); trace_dma_fence_destroy(fence); @@ -593,12 +601,12 @@ void dma_fence_release(struct kref *kref) spin_unlock_irqrestore(fence->lock, flags); }
- rcu_read_unlock(); - - if (fence->ops->release) - fence->ops->release(fence); + ops = rcu_dereference(fence->ops); + if (ops->release) + ops->release(fence); else dma_fence_free(fence); + rcu_read_unlock(); } EXPORT_SYMBOL(dma_fence_release);
@@ -617,6 +625,7 @@ EXPORT_SYMBOL(dma_fence_free);
static bool __dma_fence_enable_signaling(struct dma_fence *fence) { + const struct dma_fence_ops *ops; bool was_set;
lockdep_assert_held(fence->lock); @@ -627,14 +636,18 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence) if (dma_fence_test_signaled_flag(fence)) return false;
- if (!was_set && fence->ops->enable_signaling) { + rcu_read_lock(); + ops = rcu_dereference(fence->ops); + if (!was_set && ops->enable_signaling) { trace_dma_fence_enable_signal(fence);
- if (!fence->ops->enable_signaling(fence)) { + if (!ops->enable_signaling(fence)) { + rcu_read_unlock(); dma_fence_signal_locked(fence); return false; } } + rcu_read_unlock();
return true; } @@ -1007,8 +1020,13 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); */ void dma_fence_set_deadline(struct dma_fence *fence, ktime_t deadline) { - if (fence->ops->set_deadline && !dma_fence_is_signaled(fence)) - fence->ops->set_deadline(fence, deadline); + const struct dma_fence_ops *ops; + + rcu_read_lock(); + ops = rcu_dereference(fence->ops); + if (ops->set_deadline && !dma_fence_is_signaled(fence)) + ops->set_deadline(fence, deadline); + rcu_read_unlock(); } EXPORT_SYMBOL(dma_fence_set_deadline);
@@ -1049,7 +1067,12 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
kref_init(&fence->refcount); - fence->ops = ops; + /* + * At first glance it is counter intuitive to protect a constant + * function pointer table by RCU, but this allows modules providing the + * function table to unload by waiting for an RCU grace period. + */ + RCU_INIT_POINTER(fence->ops, ops); INIT_LIST_HEAD(&fence->cb_list); fence->lock = lock; fence->context = context; @@ -1129,11 +1152,12 @@ EXPORT_SYMBOL(dma_fence_init64); */ const char __rcu *dma_fence_driver_name(struct dma_fence *fence) { - RCU_LOCKDEP_WARN(!rcu_read_lock_held(), - "RCU protection is required for safe access to returned string"); + const struct dma_fence_ops *ops;
+ /* RCU protection is required for safe access to returned string */ + ops = rcu_dereference(fence->ops); if (!dma_fence_test_signaled_flag(fence)) - return fence->ops->get_driver_name(fence); + return ops->get_driver_name(fence); else return "detached-driver"; } @@ -1161,11 +1185,12 @@ EXPORT_SYMBOL(dma_fence_driver_name); */ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence) { - RCU_LOCKDEP_WARN(!rcu_read_lock_held(), - "RCU protection is required for safe access to returned string"); + const struct dma_fence_ops *ops;
+ /* RCU protection is required for safe access to returned string */ + ops = rcu_dereference(fence->ops); if (!dma_fence_test_signaled_flag(fence)) - return fence->ops->get_timeline_name(fence); + return ops->get_timeline_name(fence); else return "signaled-timeline"; } diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index d4c92fd35092..eea674acdfa6 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -66,7 +66,7 @@ struct seq_file; */ struct dma_fence { spinlock_t *lock; - const struct dma_fence_ops *ops; + const struct dma_fence_ops __rcu *ops; /* * We clear the callback list on kref_put so that by the time we * release the fence it is unused. No one should be adding to the @@ -218,6 +218,10 @@ struct dma_fence_ops { * timed out. Can also return other error values on custom implementations, * which should be treated as if the fence is signaled. For example a hardware * lockup could be reported like that. + * + * Implementing this callback prevents the fence from detaching after + * signaling and so it is mandatory for the module providing the + * dma_fence_ops to stay loaded as long as the dma_fence exists. */ signed long (*wait)(struct dma_fence *fence, bool intr, signed long timeout); @@ -229,6 +233,13 @@ struct dma_fence_ops { * Can be called from irq context. This callback is optional. If it is * NULL, then dma_fence_free() is instead called as the default * implementation. + * + * Implementing this callback prevents the fence from detaching after + * signaling and so it is mandatory for the module providing the + * dma_fence_ops to stay loaded as long as the dma_fence exists. + * + * If the callback is implemented the memory backing the dma_fence + * object must be freed RCU safe. */ void (*release)(struct dma_fence *fence);
@@ -439,13 +450,19 @@ dma_fence_test_signaled_flag(struct dma_fence *fence) static inline bool dma_fence_is_signaled_locked(struct dma_fence *fence) { + const struct dma_fence_ops *ops; + if (dma_fence_test_signaled_flag(fence)) return true;
- if (fence->ops->signaled && fence->ops->signaled(fence)) { + rcu_read_lock(); + ops = rcu_dereference(fence->ops); + if (ops->signaled && ops->signaled(fence)) { + rcu_read_unlock(); dma_fence_signal_locked(fence); return true; } + rcu_read_unlock();
return false; } @@ -469,13 +486,19 @@ dma_fence_is_signaled_locked(struct dma_fence *fence) static inline bool dma_fence_is_signaled(struct dma_fence *fence) { + const struct dma_fence_ops *ops; + if (dma_fence_test_signaled_flag(fence)) return true;
- if (fence->ops->signaled && fence->ops->signaled(fence)) { + rcu_read_lock(); + ops = rcu_dereference(fence->ops); + if (ops->signaled && ops->signaled(fence)) { + rcu_read_unlock(); dma_fence_signal(fence); return true; } + rcu_read_unlock();
return false; }
Hi Christian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm-xe/drm-xe-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-tip/drm-tip next-20260114] [cannot apply to drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master v6.19-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-d... base: https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next patch link: https://lore.kernel.org/r/20260113152125.47380-2-christian.koenig%40amd.com patch subject: [PATCH 01/10] dma-buf: protected fence ops by RCU v5 config: x86_64-randconfig-122-20260114 (https://download.01.org/0day-ci/archive/20260114/202601141704.EpvIhy4M-lkp@i...) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260114/202601141704.EpvIhy4M-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202601141704.EpvIhy4M-lkp@intel.com/
sparse warnings: (new ones prefixed by >>) drivers/dma-buf/dma-fence-unwrap.c: note: in included file:
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * -- drivers/dma-buf/dma-fence-array.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * -- drivers/dma-buf/dma-fence-chain.c: note: in included file (through include/linux/dma-fence-chain.h):
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * -- drivers/dma-buf/dma-resv.c: note: in included file (through include/linux/dma-resv.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * -- drivers/dma-buf/dma-fence.c:1042:38: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char const [noderef] __rcu *timeline @@ got char * @@ drivers/dma-buf/dma-fence.c:1042:38: sparse: expected char const [noderef] __rcu *timeline drivers/dma-buf/dma-fence.c:1042:38: sparse: got char * drivers/dma-buf/dma-fence.c:1043:36: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected char const [noderef] __rcu *driver @@ got char * @@ drivers/dma-buf/dma-fence.c:1043:36: sparse: expected char const [noderef] __rcu *driver drivers/dma-buf/dma-fence.c:1043:36: sparse: got char * drivers/dma-buf/dma-fence.c:1160:44: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected char const [noderef] __rcu * @@ got char const * @@ drivers/dma-buf/dma-fence.c:1160:44: sparse: expected char const [noderef] __rcu * drivers/dma-buf/dma-fence.c:1160:44: sparse: got char const * drivers/dma-buf/dma-fence.c:1162:24: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected char const [noderef] __rcu * @@ got char * @@ drivers/dma-buf/dma-fence.c:1162:24: sparse: expected char const [noderef] __rcu * drivers/dma-buf/dma-fence.c:1162:24: sparse: got char * drivers/dma-buf/dma-fence.c:1193:46: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected char const [noderef] __rcu * @@ got char const * @@ drivers/dma-buf/dma-fence.c:1193:46: sparse: expected char const [noderef] __rcu * drivers/dma-buf/dma-fence.c:1193:46: sparse: got char const * drivers/dma-buf/dma-fence.c:1195:24: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected char const [noderef] __rcu * @@ got char * @@ drivers/dma-buf/dma-fence.c:1195:24: sparse: expected char const [noderef] __rcu * drivers/dma-buf/dma-fence.c:1195:24: sparse: got char * drivers/dma-buf/dma-fence.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/dma_fence.h): include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void const *driver_ptr_ @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected void const *driver_ptr_ include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void const *timeline_ptr_ @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected void const *timeline_ptr_ include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu *
include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression
include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void const *driver_ptr_ @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected void const *driver_ptr_ include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected char const *str @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected char const *str include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu * include/trace/events/dma_fence.h:12:1: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void const *timeline_ptr_ @@ got char const [noderef] __rcu * @@ include/trace/events/dma_fence.h:12:1: sparse: expected void const *timeline_ptr_ include/trace/events/dma_fence.h:12:1: sparse: got char const [noderef] __rcu *
include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression include/trace/events/dma_fence.h:42:1: sparse: sparse: dereference of noderef expression
--
drivers/gpu/drm/drm_crtc.c:161:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/drm_crtc.c:161:9: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/drm_crtc.c:161:9: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/drm_syncobj.c: note: in included file (through include/linux/sync_file.h):
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/ttm/ttm_bo.c:1203:31: sparse: sparse: symbol 'ttm_swap_ops' was not declared. Should it be static?
drivers/gpu/drm/ttm/ttm_bo.c:226:27: sparse: sparse: dereference of noderef expression
-- drivers/gpu/drm/scheduler/sched_fence.c:241:1: sparse: sparse: bad integer constant expression drivers/gpu/drm/scheduler/sched_fence.c:241:1: sparse: sparse: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte" drivers/gpu/drm/scheduler/sched_fence.c:242:1: sparse: sparse: bad integer constant expression drivers/gpu/drm/scheduler/sched_fence.c:242:1: sparse: sparse: static assertion failed: "MODULE_INFO(file, ...) contains embedded NUL byte" drivers/gpu/drm/scheduler/sched_fence.c:242:1: sparse: sparse: bad integer constant expression drivers/gpu/drm/scheduler/sched_fence.c:242:1: sparse: sparse: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
drivers/gpu/drm/scheduler/sched_fence.c:198:20: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/scheduler/sched_fence.c:198:20: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/scheduler/sched_fence.c:198:20: sparse: struct dma_fence_ops const * drivers/gpu/drm/scheduler/sched_fence.c:201:20: sparse: sparse: incompatible types in comparison expression (different address spaces): drivers/gpu/drm/scheduler/sched_fence.c:201:20: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/scheduler/sched_fence.c:201:20: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/radeon/radeon_fence.c:73:40: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int volatile [usertype] @@ got restricted __le32 [usertype] @@ drivers/gpu/drm/radeon/radeon_fence.c:73:40: sparse: expected unsigned int volatile [usertype] drivers/gpu/drm/radeon/radeon_fence.c:73:40: sparse: got restricted __le32 [usertype] drivers/gpu/drm/radeon/radeon_fence.c:95:31: sparse: sparse: cast to restricted __le32 drivers/gpu/drm/radeon/radeon_fence.c: note: in included file:
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/radeon/radeon_display.c: note: in included file:
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/radeon/radeon_sync.c: note: in included file:
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/radeon/radeon.h:2492:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c: note: in included file (through drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h, drivers/gpu/drm/amd/amdgpu/amdgpu.h): drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:499:49: sparse: sparse: static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB" drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c: note: in included file (through include/linux/dma-fence-chain.h):
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * --
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c:93:20: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c:93:20: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c:93:20: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/i915/gem/i915_gem_busy.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/gem/i915_gem_busy.c: note: in included file (through drivers/gpu/drm/i915/gt/intel_engine.h):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/gem/i915_gem_busy.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/gem/i915_gem_busy.c: note: in included file (through drivers/gpu/drm/i915/gt/intel_engine.h):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/i915/gem/i915_gem_wait.c: note: in included file (through drivers/gpu/drm/i915/gt/intel_engine.h):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const *
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/gem/i915_gem_wait.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/i915/i915_request.c: note: in included file (through include/linux/dma-fence-array.h):
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const *
include/linux/dma-fence.h:717:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:717:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/i915_request.c: note: in included file (through drivers/gpu/drm/i915/i915_active.h, drivers/gpu/drm/i915/gt/intel_context.h, ...):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/i915_request.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/i915_request.c: note: in included file (through drivers/gpu/drm/i915/i915_active.h, drivers/gpu/drm/i915/gt/intel_context.h, ...):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/i915_request.c: note: in included file (through include/linux/dma-fence-array.h): include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * include/linux/dma-fence.h:706:27: sparse: sparse: incompatible types in comparison expression (different address spaces): include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const [noderef] __rcu * include/linux/dma-fence.h:706:27: sparse: struct dma_fence_ops const * drivers/gpu/drm/i915/i915_request.c: note: in included file (through drivers/gpu/drm/i915/i915_active.h, drivers/gpu/drm/i915/gt/intel_context.h, ...):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const * -- drivers/gpu/drm/i915/display/intel_display_rps.c: note: in included file (through drivers/gpu/drm/i915/display/intel_display_types.h): include/linux/pwm.h:13:1: sparse: sparse: bad integer constant expression include/linux/pwm.h:13:1: sparse: sparse: static assertion failed: "MODULE_INFO(import_ns, ...) contains embedded NUL byte" drivers/gpu/drm/i915/display/intel_display_rps.c: note: in included file (through drivers/gpu/drm/i915/gt/intel_engine.h, drivers/gpu/drm/i915/i915_drv.h):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const [noderef] __rcu * drivers/gpu/drm/i915/i915_request.h:369:27: sparse: struct dma_fence_ops const *
vim +717 include/linux/dma-fence.h
976b6d97c62347 Christian König 2022-01-19 708 976b6d97c62347 Christian König 2022-01-19 709 /** 976b6d97c62347 Christian König 2022-01-19 710 * dma_fence_is_chain - check if a fence is from the chain subclass 976b6d97c62347 Christian König 2022-01-19 711 * @fence: the fence to test 976b6d97c62347 Christian König 2022-01-19 712 * 976b6d97c62347 Christian König 2022-01-19 713 * Return true if it is a dma_fence_chain and false otherwise. 976b6d97c62347 Christian König 2022-01-19 714 */ 976b6d97c62347 Christian König 2022-01-19 715 static inline bool dma_fence_is_chain(struct dma_fence *fence) 976b6d97c62347 Christian König 2022-01-19 716 { 976b6d97c62347 Christian König 2022-01-19 @717 return fence->ops == &dma_fence_chain_ops; 976b6d97c62347 Christian König 2022-01-19 718 } 976b6d97c62347 Christian König 2022-01-19 719
Some driver use fence->ops to test if a fence was initialized or not. The problem is that this utilizes internal behavior of the dma_fence implementation.
So better abstract that into a function.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 13 +++++++------ drivers/gpu/drm/qxl/qxl_release.c | 2 +- include/linux/dma-fence.h | 12 ++++++++++++ 3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 0a0dcbf0798d..b97f90bbe8b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -278,9 +278,10 @@ void amdgpu_job_free_resources(struct amdgpu_job *job) unsigned i;
/* Check if any fences were initialized */ - if (job->base.s_fence && job->base.s_fence->finished.ops) + if (job->base.s_fence && + dma_fence_is_initialized(&job->base.s_fence->finished)) f = &job->base.s_fence->finished; - else if (job->hw_fence && job->hw_fence->base.ops) + else if (dma_fence_is_initialized(&job->hw_fence->base)) f = &job->hw_fence->base; else f = NULL; @@ -297,11 +298,11 @@ static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
amdgpu_sync_free(&job->explicit_sync);
- if (job->hw_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_fence->base)) dma_fence_put(&job->hw_fence->base); else kfree(job->hw_fence); - if (job->hw_vm_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_vm_fence->base)) dma_fence_put(&job->hw_vm_fence->base); else kfree(job->hw_vm_fence); @@ -335,11 +336,11 @@ void amdgpu_job_free(struct amdgpu_job *job) if (job->gang_submit != &job->base.s_fence->scheduled) dma_fence_put(job->gang_submit);
- if (job->hw_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_fence->base)) dma_fence_put(&job->hw_fence->base); else kfree(job->hw_fence); - if (job->hw_vm_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_vm_fence->base)) dma_fence_put(&job->hw_vm_fence->base); else kfree(job->hw_vm_fence); diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index 7b3c9a6016db..b38ae0b25f3c 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -146,7 +146,7 @@ qxl_release_free(struct qxl_device *qdev, idr_remove(&qdev->release_idr, release->id); spin_unlock(&qdev->release_idr_lock);
- if (release->base.ops) { + if (dma_fence_is_initialized(&release->base)) { WARN_ON(list_empty(&release->bos)); qxl_release_free_list(release);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index eea674acdfa6..371aa8ecf18e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -274,6 +274,18 @@ void dma_fence_release(struct kref *kref); void dma_fence_free(struct dma_fence *fence); void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
+/** + * dma_fence_is_initialized - test if fence was initialized + * @fence: fence to test + * + * Return: True if fence was initialized, false otherwise. Works correctly only + * when memory backing the fence structure is zero initialized on allocation. + */ +static inline bool dma_fence_is_initialized(struct dma_fence *fence) +{ + return fence && !!fence->ops; +} + /** * dma_fence_put - decreases refcount of the fence * @fence: fence to reduce refcount of
When neither a release nor a wait backend ops is specified it is possible to let the dma_fence live on independently of the module who issued it.
This makes it possible to unload drivers and only wait for all their fences to signal.
v2: fix typo in comment
Signed-off-by: Christian König christian.koenig@amd.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com Reviewed-by: Philipp Stanner phasta@kernel.org --- drivers/dma-buf/dma-fence.c | 16 ++++++++++++---- include/linux/dma-fence.h | 10 +++++++--- 2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index e3e74f98f58d..481f1cd9aae2 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -371,6 +371,14 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence, &fence->flags))) return;
+ /* + * When neither a release nor a wait operation is specified set the ops + * pointer to NULL to allow the fence structure to become independent + * from who originally issued it. + */ + if (!fence->ops->release && !fence->ops->wait) + RCU_INIT_POINTER(fence->ops, NULL); + /* Stash the cb_list before replacing it with the timestamp */ list_replace(&fence->cb_list, &cb_list);
@@ -537,7 +545,7 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) rcu_read_lock(); ops = rcu_dereference(fence->ops); trace_dma_fence_wait_start(fence); - if (ops->wait) { + if (ops && ops->wait) { /* * Implementing the wait ops is deprecated and not supported for * issuer independent fences, so it is ok to use the ops outside @@ -602,7 +610,7 @@ void dma_fence_release(struct kref *kref) }
ops = rcu_dereference(fence->ops); - if (ops->release) + if (ops && ops->release) ops->release(fence); else dma_fence_free(fence); @@ -638,7 +646,7 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
rcu_read_lock(); ops = rcu_dereference(fence->ops); - if (!was_set && ops->enable_signaling) { + if (!was_set && ops && ops->enable_signaling) { trace_dma_fence_enable_signal(fence);
if (!ops->enable_signaling(fence)) { @@ -1024,7 +1032,7 @@ void dma_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
rcu_read_lock(); ops = rcu_dereference(fence->ops); - if (ops->set_deadline && !dma_fence_is_signaled(fence)) + if (ops && ops->set_deadline && !dma_fence_is_signaled(fence)) ops->set_deadline(fence, deadline); rcu_read_unlock(); } diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 371aa8ecf18e..9bacf8855380 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -283,7 +283,11 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); */ static inline bool dma_fence_is_initialized(struct dma_fence *fence) { - return fence && !!fence->ops; + /* + * fence->ops might be set to NULL during signaling, but that will also + * set the signaled flag. + */ + return fence && (!!fence->ops || !!fence->flags); }
/** @@ -469,7 +473,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
rcu_read_lock(); ops = rcu_dereference(fence->ops); - if (ops->signaled && ops->signaled(fence)) { + if (ops && ops->signaled && ops->signaled(fence)) { rcu_read_unlock(); dma_fence_signal_locked(fence); return true; @@ -505,7 +509,7 @@ dma_fence_is_signaled(struct dma_fence *fence)
rcu_read_lock(); ops = rcu_dereference(fence->ops); - if (ops->signaled && ops->signaled(fence)) { + if (ops && ops->signaled && ops->signaled(fence)) { rcu_read_unlock(); dma_fence_signal(fence); return true;
Add dma_fence_lock_irqsafe() and dma_fence_unlock_irqrestore() wrappers and mechanically apply them everywhere.
Just a pre-requisite cleanup for a follow up patch.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/dma-buf/dma-fence.c | 44 +++++++++++------------- drivers/dma-buf/st-dma-fence.c | 6 ++-- drivers/dma-buf/sw_sync.c | 14 ++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 4 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +-- drivers/gpu/drm/nouveau/nouveau_drm.c | 5 +-- drivers/gpu/drm/scheduler/sched_fence.c | 6 ++-- drivers/gpu/drm/xe/xe_sched_job.c | 4 +-- include/linux/dma-fence.h | 20 +++++++++++ 9 files changed, 63 insertions(+), 44 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 481f1cd9aae2..be072b7ba10b 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -412,9 +412,9 @@ void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp) if (WARN_ON(!fence)) return;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); dma_fence_signal_timestamp_locked(fence, timestamp); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); } EXPORT_SYMBOL(dma_fence_signal_timestamp);
@@ -473,9 +473,9 @@ bool dma_fence_check_and_signal(struct dma_fence *fence) unsigned long flags; bool ret;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); ret = dma_fence_check_and_signal_locked(fence); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
return ret; } @@ -501,9 +501,9 @@ void dma_fence_signal(struct dma_fence *fence)
tmp = dma_fence_begin_signalling();
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); dma_fence_signal_timestamp_locked(fence, ktime_get()); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
dma_fence_end_signalling(tmp); } @@ -603,10 +603,10 @@ void dma_fence_release(struct kref *kref) * don't leave chains dangling. We set the error flag first * so that the callbacks know this signal is due to an error. */ - spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); fence->error = -EDEADLK; dma_fence_signal_locked(fence); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); }
ops = rcu_dereference(fence->ops); @@ -672,9 +672,9 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence) { unsigned long flags;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); __dma_fence_enable_signaling(fence); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); } EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
@@ -714,8 +714,7 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, return -ENOENT; }
- spin_lock_irqsave(fence->lock, flags); - + dma_fence_lock_irqsave(fence, flags); if (__dma_fence_enable_signaling(fence)) { cb->func = func; list_add_tail(&cb->node, &fence->cb_list); @@ -723,8 +722,7 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, INIT_LIST_HEAD(&cb->node); ret = -ENOENT; } - - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
return ret; } @@ -747,9 +745,9 @@ int dma_fence_get_status(struct dma_fence *fence) unsigned long flags; int status;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); status = dma_fence_get_status_locked(fence); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
return status; } @@ -779,13 +777,11 @@ dma_fence_remove_callback(struct dma_fence *fence, struct dma_fence_cb *cb) unsigned long flags; bool ret;
- spin_lock_irqsave(fence->lock, flags); - + dma_fence_lock_irqsave(fence, flags); ret = !list_empty(&cb->node); if (ret) list_del_init(&cb->node); - - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
return ret; } @@ -824,7 +820,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) unsigned long flags; signed long ret = timeout ? timeout : 1;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags);
if (dma_fence_test_signaled_flag(fence)) goto out; @@ -848,11 +844,11 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) __set_current_state(TASK_INTERRUPTIBLE); else __set_current_state(TASK_UNINTERRUPTIBLE); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
ret = schedule_timeout(ret);
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); if (ret > 0 && intr && signal_pending(current)) ret = -ERESTARTSYS; } @@ -862,7 +858,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) __set_current_state(TASK_RUNNING);
out: - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); return ret; } EXPORT_SYMBOL(dma_fence_default_wait); diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index 73ed6fd48a13..5d0d9abc6e21 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -410,8 +410,10 @@ struct race_thread {
static void __wait_for_callbacks(struct dma_fence *f) { - spin_lock_irq(f->lock); - spin_unlock_irq(f->lock); + unsigned long flags; + + dma_fence_lock_irqsave(f, flags); + dma_fence_unlock_irqrestore(f, flags); }
static int thread_signal_callback(void *arg) diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 6f09d13be6b6..4c81a37dd682 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -156,12 +156,12 @@ static void timeline_fence_release(struct dma_fence *fence) struct sync_timeline *parent = dma_fence_parent(fence); unsigned long flags;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); if (!list_empty(&pt->link)) { list_del(&pt->link); rb_erase(&pt->node, &parent->pt_tree); } - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
sync_timeline_put(parent); dma_fence_free(fence); @@ -179,7 +179,7 @@ static void timeline_fence_set_deadline(struct dma_fence *fence, ktime_t deadlin struct sync_pt *pt = dma_fence_to_sync_pt(fence); unsigned long flags;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); if (test_bit(SW_SYNC_HAS_DEADLINE_BIT, &fence->flags)) { if (ktime_before(deadline, pt->deadline)) pt->deadline = deadline; @@ -187,7 +187,7 @@ static void timeline_fence_set_deadline(struct dma_fence *fence, ktime_t deadlin pt->deadline = deadline; __set_bit(SW_SYNC_HAS_DEADLINE_BIT, &fence->flags); } - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); }
static const struct dma_fence_ops timeline_fence_ops = { @@ -431,13 +431,13 @@ static int sw_sync_ioctl_get_deadline(struct sync_timeline *obj, unsigned long a goto put_fence; }
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); if (!test_bit(SW_SYNC_HAS_DEADLINE_BIT, &fence->flags)) { ret = -ENOENT; goto unlock; } data.deadline_ns = ktime_to_ns(pt->deadline); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
dma_fence_put(fence);
@@ -450,7 +450,7 @@ static int sw_sync_ioctl_get_deadline(struct sync_timeline *obj, unsigned long a return 0;
unlock: - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags); put_fence: dma_fence_put(fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index c596b6df2e2d..60922463b415 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -474,10 +474,10 @@ bool amdgpu_ring_soft_recovery(struct amdgpu_ring *ring, unsigned int vmid, if (amdgpu_sriov_vf(ring->adev) || !ring->funcs->soft_recovery || !fence) return false;
- spin_lock_irqsave(fence->lock, flags); + dma_fence_lock_irqsave(fence, flags); if (!dma_fence_is_signaled_locked(fence)) dma_fence_set_error(fence, -ENODATA); - spin_unlock_irqrestore(fence->lock, flags); + dma_fence_unlock_irqrestore(fence, flags);
while (!dma_fence_is_signaled(fence) && ktime_to_ns(ktime_sub(deadline, ktime_get())) > 0) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index c362d4dfb5bb..ea3c187c474d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -2784,8 +2784,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) dma_fence_put(vm->last_unlocked); dma_fence_wait(vm->last_tlb_flush, false); /* Make sure that all fence callbacks have completed */ - spin_lock_irqsave(vm->last_tlb_flush->lock, flags); - spin_unlock_irqrestore(vm->last_tlb_flush->lock, flags); + dma_fence_lock_irqsave(vm->last_tlb_flush, flags); + dma_fence_unlock_irqrestore(vm->last_tlb_flush, flags); dma_fence_put(vm->last_tlb_flush);
list_for_each_entry_safe(mapping, tmp, &vm->freed, list) { diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 1527b801f013..ec4dfa3ea725 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -156,12 +156,13 @@ nouveau_name(struct drm_device *dev) static inline bool nouveau_cli_work_ready(struct dma_fence *fence) { + unsigned long flags; bool ret = true;
- spin_lock_irq(fence->lock); + dma_fence_lock_irqsave(fence, flags); if (!dma_fence_is_signaled_locked(fence)) ret = false; - spin_unlock_irq(fence->lock); + dma_fence_unlock_irqrestore(fence, flags);
if (ret == true) dma_fence_put(fence); diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c index 9391d6f0dc01..724d77694246 100644 --- a/drivers/gpu/drm/scheduler/sched_fence.c +++ b/drivers/gpu/drm/scheduler/sched_fence.c @@ -156,19 +156,19 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, struct dma_fence *parent; unsigned long flags;
- spin_lock_irqsave(&fence->lock, flags); + dma_fence_lock_irqsave(f, flags);
/* If we already have an earlier deadline, keep it: */ if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && ktime_before(fence->deadline, deadline)) { - spin_unlock_irqrestore(&fence->lock, flags); + dma_fence_unlock_irqrestore(f, flags); return; }
fence->deadline = deadline; set_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags);
- spin_unlock_irqrestore(&fence->lock, flags); + dma_fence_unlock_irqrestore(f, flags);
/* * smp_load_aquire() to ensure that if we are racing another diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c index cb674a322113..9a43ed689e65 100644 --- a/drivers/gpu/drm/xe/xe_sched_job.c +++ b/drivers/gpu/drm/xe/xe_sched_job.c @@ -189,11 +189,11 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error) unsigned long irq_flags; bool signaled;
- spin_lock_irqsave(fence->lock, irq_flags); + dma_fence_lock_irqsave(fence, irq_flags); signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags); if (!signaled) dma_fence_set_error(fence, error); - spin_unlock_irqrestore(fence->lock, irq_flags); + dma_fence_unlock_irqrestore(fence, irq_flags);
return signaled; } diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 9bacf8855380..a208e8841315 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -378,6 +378,26 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) } while (1); }
+/** + * dma_fence_lock_irqsave - irqsave lock the fence + * @fence: the fence to lock + * @flags: where to store the CPU flags. + * + * Lock the fence, preventing it from changing to the signaled state. + */ +#define dma_fence_lock_irqsave(fence, flags) \ + spin_lock_irqsave(fence->lock, flags) + +/** + * dma_fence_unlock_irqrestore - unlock the fence and irqrestore + * @fence: the fence to unlock + * @flags the CPU flags to restore + * + * Unlock the fence, allowing it to change it's state to signaled again. + */ +#define dma_fence_unlock_irqrestore(fence, flags) \ + spin_unlock_irqrestore(fence->lock, flags) + #ifdef CONFIG_LOCKDEP bool dma_fence_begin_signalling(void); void dma_fence_end_signalling(bool cookie);
Implement per-fence spinlocks, allowing implementations to not give an external spinlock to protect the fence internal statei. Instead a spinlock embedded into the fence structure itself is used in this case.
Shared spinlocks have the problem that implementations need to guarantee that the lock live at least as long all fences referencing them.
Using a per-fence spinlock allows completely decoupling spinlock producer and consumer life times, simplifying the handling in most use cases.
v2: improve naming, coverage and function documentation v3: fix one additional locking in the selftests v4: separate out some changes to make the patch smaller, fix one amdgpu crash found by CI systems
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/dma-buf/dma-fence.c | 25 +++++++++++++++++------- drivers/dma-buf/sync_debug.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +- drivers/gpu/drm/drm_crtc.c | 2 +- drivers/gpu/drm/drm_writeback.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fence.c | 3 ++- drivers/gpu/drm/qxl/qxl_release.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 ++- drivers/gpu/drm/xe/xe_hw_fence.c | 3 ++- include/linux/dma-fence.h | 26 +++++++++++++++++++++---- 10 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index be072b7ba10b..b3c1fb990621 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -343,7 +343,6 @@ void __dma_fence_might_wait(void) } #endif
- /** * dma_fence_signal_timestamp_locked - signal completion of a fence * @fence: the fence to signal @@ -365,7 +364,7 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence, struct dma_fence_cb *cur, *tmp; struct list_head cb_list;
- lockdep_assert_held(fence->lock); + lockdep_assert_held(dma_fence_spinlock(fence));
if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))) @@ -636,7 +635,7 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence) const struct dma_fence_ops *ops; bool was_set;
- lockdep_assert_held(fence->lock); + lockdep_assert_held(dma_fence_spinlock(fence));
was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags); @@ -1067,7 +1066,6 @@ static void __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno, unsigned long flags) { - BUG_ON(!lock); BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
kref_init(&fence->refcount); @@ -1078,10 +1076,15 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, */ RCU_INIT_POINTER(fence->ops, ops); INIT_LIST_HEAD(&fence->cb_list); - fence->lock = lock; fence->context = context; fence->seqno = seqno; fence->flags = flags; + if (lock) { + fence->extern_lock = lock; + } else { + spin_lock_init(&fence->inline_lock); + fence->flags |= BIT(DMA_FENCE_FLAG_INLINE_LOCK_BIT); + } fence->error = 0;
trace_dma_fence_init(fence); @@ -1091,7 +1094,7 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * dma_fence_init - Initialize a custom fence. * @fence: the fence to initialize * @ops: the dma_fence_ops for operations on this fence - * @lock: the irqsafe spinlock to use for locking this fence + * @lock: optional irqsafe spinlock to use for locking this fence * @context: the execution context this fence is run on * @seqno: a linear increasing sequence number for this context * @@ -1101,6 +1104,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, * * context and seqno are used for easy comparison between fences, allowing * to check which fence is later by simply using dma_fence_later(). + * + * It is strongly discouraged to provide an external lock. This is only allowed + * for legacy use cases when multiple fences need to be prevented from + * signaling out of order. */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, @@ -1114,7 +1121,7 @@ EXPORT_SYMBOL(dma_fence_init); * dma_fence_init64 - Initialize a custom fence with 64-bit seqno support. * @fence: the fence to initialize * @ops: the dma_fence_ops for operations on this fence - * @lock: the irqsafe spinlock to use for locking this fence + * @lock: optional irqsafe spinlock to use for locking this fence * @context: the execution context this fence is run on * @seqno: a linear increasing sequence number for this context * @@ -1124,6 +1131,10 @@ EXPORT_SYMBOL(dma_fence_init); * * Context and seqno are used for easy comparison between fences, allowing * to check which fence is later by simply using dma_fence_later(). + * + * It is strongly discouraged to provide an external lock. This is only allowed + * for legacy use cases when multiple fences need to be prevented from + * signaling out of order. */ void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, diff --git a/drivers/dma-buf/sync_debug.h b/drivers/dma-buf/sync_debug.h index 02af347293d0..c49324505b20 100644 --- a/drivers/dma-buf/sync_debug.h +++ b/drivers/dma-buf/sync_debug.h @@ -47,7 +47,7 @@ struct sync_timeline {
static inline struct sync_timeline *dma_fence_parent(struct dma_fence *fence) { - return container_of(fence->lock, struct sync_timeline, lock); + return container_of(fence->extern_lock, struct sync_timeline, lock); }
/** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 15d757c016cb..c74fa1821721 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -636,7 +636,7 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm) * sure that the dma_fence structure isn't freed up. */ rcu_read_lock(); - lock = vm->last_tlb_flush->lock; + lock = dma_fence_spinlock(vm->last_tlb_flush); rcu_read_unlock();
spin_lock_irqsave(lock, flags); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index a7797d260f1e..17472915842f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -159,7 +159,7 @@ static const struct dma_fence_ops drm_crtc_fence_ops; static struct drm_crtc *fence_to_crtc(struct dma_fence *fence) { BUG_ON(fence->ops != &drm_crtc_fence_ops); - return container_of(fence->lock, struct drm_crtc, fence_lock); + return container_of(fence->extern_lock, struct drm_crtc, fence_lock); }
static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence) diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 95b8a2e4bda6..624a4e8b6c99 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -81,7 +81,7 @@ * From userspace, this property will always read as zero. */
-#define fence_to_wb_connector(x) container_of(x->lock, \ +#define fence_to_wb_connector(x) container_of(x->extern_lock, \ struct drm_writeback_connector, \ fence_lock)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c index 4a193b7d6d9e..c282c94138b2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.c +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c @@ -41,7 +41,8 @@ static const struct dma_fence_ops nouveau_fence_ops_legacy; static inline struct nouveau_fence_chan * nouveau_fctx(struct nouveau_fence *fence) { - return container_of(fence->base.lock, struct nouveau_fence_chan, lock); + return container_of(fence->base.extern_lock, struct nouveau_fence_chan, + lock); }
static bool diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index b38ae0b25f3c..e2b95a97e90a 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -62,7 +62,8 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr, struct qxl_device *qdev; unsigned long cur, end = jiffies + timeout;
- qdev = container_of(fence->lock, struct qxl_device, release_lock); + qdev = container_of(fence->extern_lock, struct qxl_device, + release_lock);
if (!wait_event_timeout(qdev->release_event, (dma_fence_is_signaled(fence) || diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index 00be92da5509..621aa0aa8406 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -47,7 +47,8 @@ struct vmw_event_fence_action { static struct vmw_fence_manager * fman_from_fence(struct vmw_fence_obj *fence) { - return container_of(fence->base.lock, struct vmw_fence_manager, lock); + return container_of(fence->base.extern_lock, struct vmw_fence_manager, + lock); }
static void vmw_fence_obj_destroy(struct dma_fence *f) diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c index f6057456e460..3d89c660ea73 100644 --- a/drivers/gpu/drm/xe/xe_hw_fence.c +++ b/drivers/gpu/drm/xe/xe_hw_fence.c @@ -142,7 +142,8 @@ static struct xe_hw_fence *to_xe_hw_fence(struct dma_fence *fence);
static struct xe_hw_fence_irq *xe_hw_fence_irq(struct xe_hw_fence *fence) { - return container_of(fence->dma.lock, struct xe_hw_fence_irq, lock); + return container_of(fence->dma.extern_lock, struct xe_hw_fence_irq, + lock); }
static const char *xe_hw_fence_get_driver_name(struct dma_fence *dma_fence) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index a208e8841315..5f437af07b2e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -34,7 +34,8 @@ struct seq_file; * @ops: dma_fence_ops associated with this fence * @rcu: used for releasing fence with kfree_rcu * @cb_list: list of all callbacks to call - * @lock: spin_lock_irqsave used for locking + * @extern_lock: external spin_lock_irqsave used for locking + * @inline_lock: alternative internal spin_lock_irqsave used for locking * @context: execution context this fence belongs to, returned by * dma_fence_context_alloc() * @seqno: the sequence number of this fence inside the execution context, @@ -48,6 +49,7 @@ struct seq_file; * atomic ops (bit_*), so taking the spinlock will not be needed most * of the time. * + * DMA_FENCE_FLAG_INLINE_LOCK_BIT - use inline spinlock instead of external one * DMA_FENCE_FLAG_SIGNALED_BIT - fence is already signaled * DMA_FENCE_FLAG_TIMESTAMP_BIT - timestamp recorded for fence signaling * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT - enable_signaling might have been called @@ -65,7 +67,10 @@ struct seq_file; * been completed, or never called at all. */ struct dma_fence { - spinlock_t *lock; + union { + spinlock_t *extern_lock; + spinlock_t inline_lock; + }; const struct dma_fence_ops __rcu *ops; /* * We clear the callback list on kref_put so that by the time we @@ -98,6 +103,7 @@ struct dma_fence { };
enum dma_fence_flag_bits { + DMA_FENCE_FLAG_INLINE_LOCK_BIT, DMA_FENCE_FLAG_SEQNO64_BIT, DMA_FENCE_FLAG_SIGNALED_BIT, DMA_FENCE_FLAG_TIMESTAMP_BIT, @@ -378,6 +384,18 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) } while (1); }
+/** + * dma_fence_spinlock - return pointer to the spinlock protecting the fence + * @fence: the fence to get the lock from + * + * Return either the pointer to the embedded or the external spin lock. + */ +static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence) +{ + return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ? + &fence->inline_lock : fence->extern_lock; +} + /** * dma_fence_lock_irqsave - irqsave lock the fence * @fence: the fence to lock @@ -386,7 +404,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) * Lock the fence, preventing it from changing to the signaled state. */ #define dma_fence_lock_irqsave(fence, flags) \ - spin_lock_irqsave(fence->lock, flags) + spin_lock_irqsave(dma_fence_spinlock(fence), flags)
/** * dma_fence_unlock_irqrestore - unlock the fence and irqrestore @@ -396,7 +414,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) * Unlock the fence, allowing it to change it's state to signaled again. */ #define dma_fence_unlock_irqrestore(fence, flags) \ - spin_unlock_irqrestore(fence->lock, flags) + spin_unlock_irqrestore(dma_fence_spinlock(fence), flags)
#ifdef CONFIG_LOCKDEP bool dma_fence_begin_signalling(void);
Hi Christian,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm-xe/drm-xe-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-tip/drm-tip next-20260114] [cannot apply to drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master v6.19-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-d... base: https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next patch link: https://lore.kernel.org/r/20260113152125.47380-6-christian.koenig%40amd.com patch subject: [PATCH 05/10] dma-buf: inline spinlock for fence protection v4 config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260114/202601141517.1B4n6bXr-lkp@i...) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260114/202601141517.1B4n6bXr-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202601141517.1B4n6bXr-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:151:29: error: no member named 'lock' in 'struct dma_fence'
151 | lockdep_assert_held(fence->lock); | ~~~~~ ^ include/linux/lockdep.h:392:46: note: expanded from macro 'lockdep_assert_held' 392 | #define lockdep_assert_held(l) do { (void)(l); } while (0) | ^ 1 error generated. --
drivers/gpu/drm/i915/i915_active.c:1048:27: error: no member named 'lock' in 'struct dma_fence'
1048 | spin_lock_irqsave(fence->lock, flags); | ~~~~~ ^ include/linux/spinlock.h:381:39: note: expanded from macro 'spin_lock_irqsave' 381 | raw_spin_lock_irqsave(spinlock_check(lock), flags); \ | ^~~~ include/linux/spinlock.h:244:34: note: expanded from macro 'raw_spin_lock_irqsave' 244 | flags = _raw_spin_lock_irqsave(lock); \ | ^~~~ drivers/gpu/drm/i915/i915_active.c:1050:26: error: no member named 'lock' in 'struct dma_fence' 1050 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ~~~~ ^ include/linux/spinlock.h:366:38: note: expanded from macro 'spin_lock_nested' 366 | raw_spin_lock_nested(spinlock_check(lock), subclass); \ | ^~~~ include/linux/spinlock.h:235:37: note: expanded from macro 'raw_spin_lock_nested' 235 | _raw_spin_lock(((void)(subclass), (lock))) | ^~~~ drivers/gpu/drm/i915/i915_active.c:1064:22: error: no member named 'lock' in 'struct dma_fence' 1064 | spin_unlock(prev->lock); | ~~~~ ^ drivers/gpu/drm/i915/i915_active.c:1067:33: error: no member named 'lock' in 'struct dma_fence' 1067 | spin_unlock_irqrestore(fence->lock, flags); | ~~~~~ ^ drivers/gpu/drm/i915/i915_active.c:1072:28: error: no member named 'lock' in 'struct dma_fence' 1072 | spin_lock_irqsave(fence->lock, flags); | ~~~~~ ^ include/linux/spinlock.h:381:39: note: expanded from macro 'spin_lock_irqsave' 381 | raw_spin_lock_irqsave(spinlock_check(lock), flags); \ | ^~~~ include/linux/spinlock.h:244:34: note: expanded from macro 'raw_spin_lock_irqsave' 244 | flags = _raw_spin_lock_irqsave(lock); \ | ^~~~ drivers/gpu/drm/i915/i915_active.c:1074:27: error: no member named 'lock' in 'struct dma_fence' 1074 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ~~~~ ^ include/linux/spinlock.h:366:38: note: expanded from macro 'spin_lock_nested' 366 | raw_spin_lock_nested(spinlock_check(lock), subclass); \ | ^~~~ include/linux/spinlock.h:235:37: note: expanded from macro 'raw_spin_lock_nested' 235 | _raw_spin_lock(((void)(subclass), (lock))) | ^~~~ drivers/gpu/drm/i915/i915_active.c:1091:21: error: no member named 'lock' in 'struct dma_fence' 1091 | spin_unlock(prev->lock); /* serialise with prev->cb_list */ | ~~~~ ^ drivers/gpu/drm/i915/i915_active.c:1094:32: error: no member named 'lock' in 'struct dma_fence' 1094 | spin_unlock_irqrestore(fence->lock, flags); | ~~~~~ ^ 8 errors generated.
vim +151 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
0152b3b3f49b36 Chris Wilson 2019-05-08 144 0152b3b3f49b36 Chris Wilson 2019-05-08 145 static void f2cb60e9a3881e Chris Wilson 2019-08-17 146 __dma_fence_signal__notify(struct dma_fence *fence, f2cb60e9a3881e Chris Wilson 2019-08-17 147 const struct list_head *list) 0152b3b3f49b36 Chris Wilson 2019-05-08 148 { 0152b3b3f49b36 Chris Wilson 2019-05-08 149 struct dma_fence_cb *cur, *tmp; 0152b3b3f49b36 Chris Wilson 2019-05-08 150 0152b3b3f49b36 Chris Wilson 2019-05-08 @151 lockdep_assert_held(fence->lock); 0152b3b3f49b36 Chris Wilson 2019-05-08 152 f2cb60e9a3881e Chris Wilson 2019-08-17 153 list_for_each_entry_safe(cur, tmp, list, node) { 0152b3b3f49b36 Chris Wilson 2019-05-08 154 INIT_LIST_HEAD(&cur->node); 0152b3b3f49b36 Chris Wilson 2019-05-08 155 cur->func(fence, cur); 0152b3b3f49b36 Chris Wilson 2019-05-08 156 } 0152b3b3f49b36 Chris Wilson 2019-05-08 157 } 0152b3b3f49b36 Chris Wilson 2019-05-08 158
Hi Christian,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm-xe/drm-xe-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-tip/drm-tip next-20260114] [cannot apply to drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master v6.19-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-d... base: https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next patch link: https://lore.kernel.org/r/20260113152125.47380-6-christian.koenig%40amd.com patch subject: [PATCH 05/10] dma-buf: inline spinlock for fence protection v4 config: x86_64-buildonly-randconfig-004-20260114 (https://download.01.org/0day-ci/archive/20260114/202601141412.WQDwevjM-lkp@i...) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260114/202601141412.WQDwevjM-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202601141412.WQDwevjM-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/include/asm/bug.h:193, from arch/x86/include/asm/alternative.h:9, from arch/x86/include/asm/segment.h:6, from arch/x86/include/asm/ptrace.h:5, from arch/x86/include/asm/math_emu.h:5, from arch/x86/include/asm/processor.h:13, from include/linux/sched.h:13, from include/linux/kthread.h:6, from drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:6: drivers/gpu/drm/i915/gt/intel_breadcrumbs.c: In function '__dma_fence_signal__notify':
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:151:34: error: 'struct dma_fence' has no member named 'lock'
151 | lockdep_assert_held(fence->lock); | ^~ include/asm-generic/bug.h:205:32: note: in definition of macro 'WARN_ON' 205 | int __ret_warn_on = !!(condition); \ | ^~~~~~~~~ include/linux/lockdep.h:285:9: note: in expansion of macro 'lockdep_assert' 285 | lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) | ^~~~~~~~~~~~~~ include/linux/lockdep.h:285:24: note: in expansion of macro 'lockdep_is_held' 285 | lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) | ^~~~~~~~~~~~~~~ drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:151:9: note: in expansion of macro 'lockdep_assert_held' 151 | lockdep_assert_held(fence->lock); | ^~~~~~~~~~~~~~~~~~~ -- In file included from include/linux/debugobjects.h:6, from drivers/gpu/drm/i915/i915_active.c:7: drivers/gpu/drm/i915/i915_active.c: In function '__i915_active_fence_set':
drivers/gpu/drm/i915/i915_active.c:1048:32: error: 'struct dma_fence' has no member named 'lock'
1048 | spin_lock_irqsave(fence->lock, flags); | ^~ include/linux/spinlock.h:244:48: note: in definition of macro 'raw_spin_lock_irqsave' 244 | flags = _raw_spin_lock_irqsave(lock); \ | ^~~~ drivers/gpu/drm/i915/i915_active.c:1048:9: note: in expansion of macro 'spin_lock_irqsave' 1048 | spin_lock_irqsave(fence->lock, flags); | ^~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_active.c:1050:38: error: 'struct dma_fence' has no member named 'lock' 1050 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ^~ include/linux/spinlock.h:221:31: note: in definition of macro 'raw_spin_lock_nested' 221 | _raw_spin_lock_nested(lock, subclass) | ^~~~ drivers/gpu/drm/i915/i915_active.c:1050:17: note: in expansion of macro 'spin_lock_nested' 1050 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ^~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_active.c:1064:41: error: 'struct dma_fence' has no member named 'lock' 1064 | spin_unlock(prev->lock); | ^~ drivers/gpu/drm/i915/i915_active.c:1067:45: error: 'struct dma_fence' has no member named 'lock' 1067 | spin_unlock_irqrestore(fence->lock, flags); | ^~ drivers/gpu/drm/i915/i915_active.c:1072:40: error: 'struct dma_fence' has no member named 'lock' 1072 | spin_lock_irqsave(fence->lock, flags); | ^~ include/linux/spinlock.h:244:48: note: in definition of macro 'raw_spin_lock_irqsave' 244 | flags = _raw_spin_lock_irqsave(lock); \ | ^~~~ drivers/gpu/drm/i915/i915_active.c:1072:17: note: in expansion of macro 'spin_lock_irqsave' 1072 | spin_lock_irqsave(fence->lock, flags); | ^~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_active.c:1074:46: error: 'struct dma_fence' has no member named 'lock' 1074 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ^~ include/linux/spinlock.h:221:31: note: in definition of macro 'raw_spin_lock_nested' 221 | _raw_spin_lock_nested(lock, subclass) | ^~~~ drivers/gpu/drm/i915/i915_active.c:1074:25: note: in expansion of macro 'spin_lock_nested' 1074 | spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING); | ^~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_active.c:1091:33: error: 'struct dma_fence' has no member named 'lock' 1091 | spin_unlock(prev->lock); /* serialise with prev->cb_list */ | ^~ drivers/gpu/drm/i915/i915_active.c:1094:37: error: 'struct dma_fence' has no member named 'lock' 1094 | spin_unlock_irqrestore(fence->lock, flags); | ^~ In file included from drivers/gpu/drm/i915/i915_active.c:1174: drivers/gpu/drm/i915/selftests/i915_active.c: In function 'active_flush':
drivers/gpu/drm/i915/selftests/i915_active.c:326:28: error: 'struct dma_fence' has no member named 'lock'
326 | spin_lock_irq(fence->lock); | ^~ drivers/gpu/drm/i915/selftests/i915_active.c:328:30: error: 'struct dma_fence' has no member named 'lock' 328 | spin_unlock_irq(fence->lock); /* serialise with fence->cb_list */ | ^~
vim +151 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
0152b3b3f49b36 Chris Wilson 2019-05-08 144 0152b3b3f49b36 Chris Wilson 2019-05-08 145 static void f2cb60e9a3881e Chris Wilson 2019-08-17 146 __dma_fence_signal__notify(struct dma_fence *fence, f2cb60e9a3881e Chris Wilson 2019-08-17 147 const struct list_head *list) 0152b3b3f49b36 Chris Wilson 2019-05-08 148 { 0152b3b3f49b36 Chris Wilson 2019-05-08 149 struct dma_fence_cb *cur, *tmp; 0152b3b3f49b36 Chris Wilson 2019-05-08 150 0152b3b3f49b36 Chris Wilson 2019-05-08 @151 lockdep_assert_held(fence->lock); 0152b3b3f49b36 Chris Wilson 2019-05-08 152 f2cb60e9a3881e Chris Wilson 2019-08-17 153 list_for_each_entry_safe(cur, tmp, list, node) { 0152b3b3f49b36 Chris Wilson 2019-05-08 154 INIT_LIST_HEAD(&cur->node); 0152b3b3f49b36 Chris Wilson 2019-05-08 155 cur->func(fence, cur); 0152b3b3f49b36 Chris Wilson 2019-05-08 156 } 0152b3b3f49b36 Chris Wilson 2019-05-08 157 } 0152b3b3f49b36 Chris Wilson 2019-05-08 158
Drop the mock_fence and the kmem_cache, instead use the inline lock and test if the ops are properly dropped after signaling.
v2: move the RCU check to the end of the test
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/dma-buf/st-dma-fence.c | 44 ++++++++-------------------------- 1 file changed, 10 insertions(+), 34 deletions(-)
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index 5d0d9abc6e21..0d9d524d79b6 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -14,43 +14,26 @@
#include "selftest.h"
-static struct kmem_cache *slab_fences; - -static struct mock_fence { - struct dma_fence base; - struct spinlock lock; -} *to_mock_fence(struct dma_fence *f) { - return container_of(f, struct mock_fence, base); -} - static const char *mock_name(struct dma_fence *f) { return "mock"; }
-static void mock_fence_release(struct dma_fence *f) -{ - kmem_cache_free(slab_fences, to_mock_fence(f)); -} - static const struct dma_fence_ops mock_ops = { .get_driver_name = mock_name, .get_timeline_name = mock_name, - .release = mock_fence_release, };
static struct dma_fence *mock_fence(void) { - struct mock_fence *f; + struct dma_fence *f;
- f = kmem_cache_alloc(slab_fences, GFP_KERNEL); + f = kmalloc(sizeof(*f), GFP_KERNEL); if (!f) return NULL;
- spin_lock_init(&f->lock); - dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0); - - return &f->base; + dma_fence_init(f, &mock_ops, NULL, 0, 0); + return f; }
static int sanitycheck(void *arg) @@ -100,6 +83,11 @@ static int test_signaling(void *arg) goto err_free; }
+ if (rcu_dereference_protected(f->ops, true)) { + pr_err("Fence ops not cleared on signal\n"); + goto err_free; + } + err = 0; err_free: dma_fence_put(f); @@ -540,19 +528,7 @@ int dma_fence(void) SUBTEST(test_stub), SUBTEST(race_signal_callback), }; - int ret;
pr_info("sizeof(dma_fence)=%zu\n", sizeof(struct dma_fence)); - - slab_fences = KMEM_CACHE(mock_fence, - SLAB_TYPESAFE_BY_RCU | - SLAB_HWCACHE_ALIGN); - if (!slab_fences) - return -ENOMEM; - - ret = subtests(tests, NULL); - - kmem_cache_destroy(slab_fences); - - return ret; + return subtests(tests, NULL); }
Using the inline lock is now the recommended way for dma_fence implementations.
So use this approach for the framework's internal fences as well.
Also saves about 4 bytes for the external spinlock.
v2: drop unecessary changes
Signed-off-by: Christian König christian.koenig@amd.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com --- drivers/dma-buf/dma-fence.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index b3c1fb990621..d1a9a1cdd64b 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -24,7 +24,6 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit); EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal); EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
-static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub;
/* @@ -123,12 +122,9 @@ static const struct dma_fence_ops dma_fence_stub_ops = {
static int __init dma_fence_init_stub(void) { - dma_fence_init(&dma_fence_stub, &dma_fence_stub_ops, - &dma_fence_stub_lock, 0, 0); - + dma_fence_init(&dma_fence_stub, &dma_fence_stub_ops, NULL, 0, 0); set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &dma_fence_stub.flags); - dma_fence_signal(&dma_fence_stub); return 0; } @@ -160,11 +156,7 @@ struct dma_fence *dma_fence_allocate_private_stub(ktime_t timestamp) if (fence == NULL) return NULL;
- dma_fence_init(fence, - &dma_fence_stub_ops, - &dma_fence_stub_lock, - 0, 0); - + dma_fence_init(fence, &dma_fence_stub_ops, NULL, 0, 0); set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags);
Using the inline lock is now the recommended way for dma_fence implementations.
So use this approach for the framework's internal fences as well.
Also saves about 4 bytes for the external spinlock.
Signed-off-by: Christian König christian.koenig@amd.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com Reviewed-by: Philipp Stanner phasta@kernel.org --- drivers/dma-buf/dma-fence-array.c | 5 ++--- include/linux/dma-fence-array.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 6657d4b30af9..c2119a8049fe 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -204,9 +204,8 @@ void dma_fence_array_init(struct dma_fence_array *array,
array->num_fences = num_fences;
- spin_lock_init(&array->lock); - dma_fence_init(&array->base, &dma_fence_array_ops, &array->lock, - context, seqno); + dma_fence_init(&array->base, &dma_fence_array_ops, NULL, context, + seqno); init_irq_work(&array->work, irq_dma_fence_array_work);
atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences); diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index 079b3dec0a16..370b3d2bba37 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -38,7 +38,6 @@ struct dma_fence_array_cb { struct dma_fence_array { struct dma_fence base;
- spinlock_t lock; unsigned num_fences; atomic_t num_pending; struct dma_fence **fences;
Using the inline lock is now the recommended way for dma_fence implementations.
So use this approach for the framework's internal fences as well.
Also saves about 4 bytes for the external spinlock.
Signed-off-by: Christian König christian.koenig@amd.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com Reviewed-by: Philipp Stanner phasta@kernel.org --- drivers/dma-buf/dma-fence-chain.c | 3 +-- include/linux/dma-fence-chain.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index a8a90acf4f34..a707792b6025 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -245,7 +245,6 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, struct dma_fence_chain *prev_chain = to_dma_fence_chain(prev); uint64_t context;
- spin_lock_init(&chain->lock); rcu_assign_pointer(chain->prev, prev); chain->fence = fence; chain->prev_seqno = 0; @@ -261,7 +260,7 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, seqno = max(prev->seqno, seqno); }
- dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock, + dma_fence_init64(&chain->base, &dma_fence_chain_ops, NULL, context, seqno);
/* diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h index 68c3c1e41014..d39ce7a2e599 100644 --- a/include/linux/dma-fence-chain.h +++ b/include/linux/dma-fence-chain.h @@ -46,7 +46,6 @@ struct dma_fence_chain { */ struct irq_work work; }; - spinlock_t lock; };
Using the inline lock is now the recommended way for dma_fence implementations.
For the scheduler fence use the inline lock for the scheduled fence part and then the lock from the scheduled fence as external lock for the finished fence.
This way there is no functional difference, except for saving the space for the separate lock.
v2: re-work the patch to avoid any functional difference
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/scheduler/sched_fence.c | 6 +++--- include/drm/gpu_scheduler.h | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c index 724d77694246..112677231f9a 100644 --- a/drivers/gpu/drm/scheduler/sched_fence.c +++ b/drivers/gpu/drm/scheduler/sched_fence.c @@ -217,7 +217,6 @@ struct drm_sched_fence *drm_sched_fence_alloc(struct drm_sched_entity *entity,
fence->owner = owner; fence->drm_client_id = drm_client_id; - spin_lock_init(&fence->lock);
return fence; } @@ -230,9 +229,10 @@ void drm_sched_fence_init(struct drm_sched_fence *fence, fence->sched = entity->rq->sched; seq = atomic_inc_return(&entity->fence_seq); dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled, - &fence->lock, entity->fence_context, seq); + NULL, entity->fence_context, seq); dma_fence_init(&fence->finished, &drm_sched_fence_ops_finished, - &fence->lock, entity->fence_context + 1, seq); + dma_fence_spinlock(&fence->scheduled), + entity->fence_context + 1, seq); }
module_init(drm_sched_fence_slab_init); diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 78e07c2507c7..ad3704685163 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -297,10 +297,6 @@ struct drm_sched_fence { * belongs to. */ struct drm_gpu_scheduler *sched; - /** - * @lock: the lock used by the scheduled and the finished fences. - */ - spinlock_t lock; /** * @owner: job owner for debugging */
linaro-mm-sig@lists.linaro.org
-
Christian König -
kernel test robot