Implement per-fence spinlocks, allowing implementations to not give an
external spinlock to protect the fence internal state. 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 lives 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
v5: improve comments
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
---
drivers/dma-buf/dma-fence.c | 21 ++++++++++++++++-----
drivers/dma-buf/sync_debug.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 | 19 +++++++++++++------
9 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index bd4ec7e26dae..39f0a4d08a2d 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
@@ -1068,7 +1067,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);
@@ -1080,10 +1078,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 | BIT(DMA_FENCE_FLAG_INITIALIZED_BIT);
+ 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);
@@ -1093,7 +1096,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
*
@@ -1103,6 +1106,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 because this couples
+ * lock and fence life time. 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,
@@ -1116,7 +1123,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
*
@@ -1126,6 +1133,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 because this couples
+ * lock and fence life time. 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/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 06b0b2aa7953..37d4ae0faf0d 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 85795082fef9..d251eec57df9 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 ae8ed15b64c5..14720623ad00 100644
--- a/drivers/gpu/drm/xe/xe_hw_fence.c
+++ b/drivers/gpu/drm/xe/xe_hw_fence.c
@@ -124,7 +124,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 086324af96c9..9ed359e38d4e 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 (deprecated)
+ * @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,
@@ -49,6 +50,7 @@ struct seq_file;
* of the time.
*
* DMA_FENCE_FLAG_INITIALIZED_BIT - fence was initialized
+ * 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
@@ -66,7 +68,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
@@ -100,6 +105,7 @@ struct dma_fence {
enum dma_fence_flag_bits {
DMA_FENCE_FLAG_INITIALIZED_BIT,
+ DMA_FENCE_FLAG_INLINE_LOCK_BIT,
DMA_FENCE_FLAG_SEQNO64_BIT,
DMA_FENCE_FLAG_SIGNALED_BIT,
DMA_FENCE_FLAG_TIMESTAMP_BIT,
@@ -381,11 +387,12 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
* dma_fence_spinlock - return pointer to the spinlock protecting the fence
* @fence: the fence to get the lock from
*
- * Return the pointer to the extern lock.
+ * Return either the pointer to the embedded or the external spin lock.
*/
static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence)
{
- return fence->lock;
+ return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ?
+ &fence->inline_lock : fence->extern_lock;
}
/**
@@ -396,7 +403,7 @@ static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence)
* 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
@@ -406,7 +413,7 @@ static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence)
* 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)
/**
* dma_fence_assert_held - lockdep assertion that fence is locked
--
2.43.0
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.
v6:
Minor style changes, re-ordered patch #1, dropped the scheduler fence
change for now
v7:
The patch adding the dma_fence_was_initialized() function was pushed
upstream individually since that is really an independent cleanup.
Fixed some missing i915 bits in patch "dma-buf: abstract fence locking".
Please review and comment,
Christian.
I want to share my experience with OPTIMISTIC HACKER GAIUS after I was scammed of my cryptocurrency. Feeling helpless, I reached out and received clear, professional guidance from the start. I explained the process, communicated consistently, and handled the situation with care. Through his expertise and persistence, my scammed crypto was successfully recovered. The support restored my confidence and eased the stress of the order. I’m grateful for the help and would recommend his services.
Website: optimistichackargaius .c o m
Email: support @ optimistichackargaius .c om
WhatsApp: +44 737 674 0569
Cryptocurrency investing has always been a passion of mine but that passion quickly turned into panic when I became the victim of a fraudulent SOL trading platform. The platform promised a fifteen percent daily return which seemed like an incredible opportunity I could not miss. I was drawn in by the sleek interface, positive reviews and supposed user testimonials. Believing I had found a breakthrough I went ahead and deposited 92000 in SOL.To my horror the platform shut down the very next day. My account was wiped clean and I had no access to my funds. Just like that my entire investment vanished and I was left in financial ruin. The days that followed were some of the darkest I have ever experienced. I struggled to pay my bills, skipped meals and found myself in a deep emotional and financial pit.Feeling defeated and ashamed I turned to a close friend and confided in them. That conversation changed everything. My friend referred me to SALVAGE ASSET RECOVERY , a specialized team that deals with cryptocurrency scam recovery. I was skeptical at first. After all, I had already been scammed once and was hesitant to trust anyone again. But desperation pushed me to give it a try. SALVAGE ASSET RECOVERY impressed me. They listened carefully to my story, explained the process in clear detail and reassured me that they would do everything possible to help. Using advanced blockchain tools and digital tracing methods they were able to locate the flow of my stolen funds.Weeks later I received the news I never thought I would hear. SALVAGE ASSET RECOVERY had successfully recovered my entire 92000 in SOL. I was overwhelmed with relief and gratitude. I could finally breathe again knowing that my hard earned money had not been lost forever. If something sounds too good to be true in the crypto world it almost always is. While I still believe in the future of blockchain I now invest with much more caution and diligence.To anyone who has fallen victim to a similar scam know that all hope is not lost. Reach out to people who can truly help. SALVAGE ASSET RECOVERY gave me a second chance and for that I will always be thankful. you can use the details provided below.
Whats app : +18476547096
Telegram : @Salvageasset
Cryptocurrency investing has always been a passion of mine but that passion quickly turned into panic when I became the victim of a fraudulent SOL trading platform. The platform promised a fifteen percent daily return which seemed like an incredible opportunity I could not miss. I was drawn in by the sleek interface, positive reviews and supposed user testimonials. Believing I had found a breakthrough I went ahead and deposited 92000 in SOL.To my horror the platform shut down the very next day. My account was wiped clean and I had no access to my funds. Just like that my entire investment vanished and I was left in financial ruin. The days that followed were some of the darkest I have ever experienced. I struggled to pay my bills, skipped meals and found myself in a deep emotional and financial pit.Feeling defeated and ashamed I turned to a close friend and confided in them. That conversation changed everything. My friend referred me to SALVAGE ASSET RECOVERY , a specialized team that deals with cryptocurrency scam recovery. I was skeptical at first. After all, I had already been scammed once and was hesitant to trust anyone again. But desperation pushed me to give it a try. SALVAGE ASSET RECOVERY impressed me. They listened carefully to my story, explained the process in clear detail and reassured me that they would do everything possible to help. Using advanced blockchain tools and digital tracing methods they were able to locate the flow of my stolen funds.Weeks later I received the news I never thought I would hear. SALVAGE ASSET RECOVERY had successfully recovered my entire 92000 in SOL. I was overwhelmed with relief and gratitude. I could finally breathe again knowing that my hard earned money had not been lost forever. If something sounds too good to be true in the crypto world it almost always is. While I still believe in the future of blockchain I now invest with much more caution and diligence.To anyone who has fallen victim to a similar scam know that all hope is not lost. Reach out to people who can truly help. SALVAGE ASSET RECOVERY gave me a second chance and for that I will always be thankful. you can use the details provided below.
Whats app : +18476547096
Telegram : @Salvageasset