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
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(a)amd.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com>
Reviewed-by: Philipp Stanner <phasta(a)kernel.org>
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
---
drivers/dma-buf/dma-fence.c | 16 ++++++++++++----
include/linux/dma-fence.h | 4 ++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index d3c4e23bf297..ae77f900c267 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
* issuers of fences who need their lifetime to be independent
@@ -603,7 +611,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);
@@ -639,7 +647,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)) {
@@ -1025,7 +1033,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 9d8a4ebe8bf7..80db7ede91de 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -472,7 +472,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;
@@ -508,7 +508,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;
--
2.43.0
Hello my name is Fred Ruben and i want to share my story with you. I've always prided myself on being financially savvy, but falling for a fake Forex platform was a harsh wake up call. I’ve been in trading for a long time, mostly sticking to traditional stock investments and the occasional dabble in options. But when the pandemic hit and I found myself with more time at home, I started reading about Forex trading. I did my research or at least I thought I did. I read articles, joined online forums, and watched YouTube tutorials. Eventually, I came across what seemed like a legitimate trading company. It had great reviews and what appeared to be a solid track record of success. The brokers on the platform were very responsive, and I quickly got assigned a personal trade assistant, who managed my portfolio and handled my trading. I started out small with a modest $10,000 investment to test the waters, and within a few days, I was seeing profits. The numbers looked great, and before I knew it, I had invested a total of $235,000. After a few months, I decided it was time to take some profits out. I was planning to withdraw to cover some expenses, and I submitted a withdrawal request through the platform but I was met with an error message. I reached out to my advisor but the friendly tone of my advisor suddenly turned cold and dismissive, encouraging me to keep retrying the attempt and then, one day, he stopped responding entirely leaving me feeling completely helpless but at that point, all I could think about was how to get my money back. Thankfully, After some desperate Google searches, I was fortunate to find SALVAGE ASSET RECOVERY, and I can’t recommend them highly enough for anyone in a similar situation. The team at SALVAGE ASSET RECOVERY walked me through the entire process. Apparently, the platform I had invested in was part of a larger network of fake trading sites that had defrauded hundreds of people and within a couple of days of tracing my funds, the team successfully recovered the total sum I had lost safely back into my bank accounts. If you’re reading this and you’re in a similar situation with a supposed online investment, quickly reach out to the team at SALVAGE ASSET RECOVERY using any of the contact information below
Telegram→ @Salvageasset
Whats +18476547096
EMAIL→ Salvageassetrecovery(a)alumni.com
On 2/13/26 15:22, Boris Brezillon wrote:
>> ---
>> drivers/dma-buf/dma-fence.c | 16 ++++++++++++----
>> include/linux/dma-fence.h | 4 ++--
>> 2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index de9bf18be3d4..ba02321bef0b 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.
>
> I think this deserves some comment in the dma_fence_ops doc, so that
> people know what to expect when they implement this interface.
There was already a warning added like ~5years ago that implementations shouldn't use the wait callback.
Completely independent of this patch set here we already had tons of trouble with it because it can't take into account when userpsace waits for multiple fences from different implementations.
It potentially was never a good idea to have in the first place, we basically only had it because radeon (and IIRC nouveau at that point) depended on it.
Regards,
Christian.
On 2/19/26 11:35, Philipp Stanner wrote:
> On Thu, 2026-02-19 at 11:23 +0100, Christian König wrote:
>> On 2/12/26 09:56, Philipp Stanner wrote:
>>>>>> @@ -454,13 +465,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)) {
>>>>>
>>>>> Maybe you can educate me a bit about RCU here – couldn't this still
>>>>> race? If the ops were unloaded before you take rcu_read_lock(),
>>>>> rcu_dereference() would give you an invalid pointer here since you
>>>>> don't check for !ops, no?
>>>>
>>>> Perfectly correct thinking, yes.
>>>>
>>>> But the check for !ops is added in patch #2 when we actually start to set ops = NULL when the fence signals.
>>>>
>>>> I intentionally separated that because it is basically the second step in making the solution to detach the fence ops from the module by RCU work.
>>>>
>>>> We could merge the two patches together, but I think the separation actually makes sense should anybody start to complain about the additional RCU overhead.
>>>>
>>>
>>> Alright, makes sense. However the above does not read correct..
>>>
>>> But then my question would be: What's the purpose of this patch, what
>>> does it solve or address atomically?
>>
>> Adding the RCU annotation and related logic, e.g. rcu_read_lock()/rcu_read_unlock()/rcu_dereference() etc...
>>
>> This allows the automated statically RCU checker to validate what we do here and point out potential mistakes.
>>
>> Additional to that should adding the rcu_read_lock() protection cause performance problems it will bisect to this patch here alone.
>
> Alright, thx for the info. Very useful
>
>>
>>> Adding RCU here does not yet change behavior and it does not solve the
>>> unloading problem, does it?
>>
>> Nope, no functional behavior change. It's purely to get the automated checkers going.
>>
>>> If it's a mere preperational step and the patches should not be merged,
>>> I'd guard the above with a simple comment like "Cleanup preparation.
>>> 'ops' can yet not be NULL, but this will be the case subsequently."
>>
>> A comment added in this patch and removed in the next one? Na, that sounds like overkill to me.
>
> ACK.
> But then lets do a normalkill by adding the info you provided above
> into the commit message, shall we? ^_^
>
> "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."
>
> This doesn't reveal what the patch is actually about, just that
> something is counter-intuitive to someone already very familiar with
> the series' intent and the code's deeper background :)
>
> "This or that about dma_fence shall be cleaned up in subsequent
> patches. To prepare for that, add … which allows the RCU checker to
> validate …"
I've already added the sentence "...As first step to solve this issue protect the fence ops by RCU." in the commit message to make it clear that this is not a full solution to the issue.
> *Philipp reads that*: ["Ah, this patch is in preparation and allows the
> RCU checker to validate everything!"]
Yeah, mentioning the RCU checker is clearly a good idea. Going to add that.
Christian.
>
> ;p
>
> P.
On 2/12/26 09:56, Philipp Stanner wrote:
>>>> @@ -454,13 +465,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)) {
>>>
>>> Maybe you can educate me a bit about RCU here – couldn't this still
>>> race? If the ops were unloaded before you take rcu_read_lock(),
>>> rcu_dereference() would give you an invalid pointer here since you
>>> don't check for !ops, no?
>>
>> Perfectly correct thinking, yes.
>>
>> But the check for !ops is added in patch #2 when we actually start to set ops = NULL when the fence signals.
>>
>> I intentionally separated that because it is basically the second step in making the solution to detach the fence ops from the module by RCU work.
>>
>> We could merge the two patches together, but I think the separation actually makes sense should anybody start to complain about the additional RCU overhead.
>>
>
> Alright, makes sense. However the above does not read correct..
>
> But then my question would be: What's the purpose of this patch, what
> does it solve or address atomically?
Adding the RCU annotation and related logic, e.g. rcu_read_lock()/rcu_read_unlock()/rcu_dereference() etc...
This allows the automated statically RCU checker to validate what we do here and point out potential mistakes.
Additional to that should adding the rcu_read_lock() protection cause performance problems it will bisect to this patch here alone.
> Adding RCU here does not yet change behavior and it does not solve the
> unloading problem, does it?
Nope, no functional behavior change. It's purely to get the automated checkers going.
> If it's a mere preperational step and the patches should not be merged,
> I'd guard the above with a simple comment like "Cleanup preparation.
> 'ops' can yet not be NULL, but this will be the case subsequently."
A comment added in this patch and removed in the next one? Na, that sounds like overkill to me.
Christian.
>
>
> P.
>