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.