Linux kernel mirror (for testing)
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel
os
linux
dma-buf: protected fence ops by RCU v8
The fence ops of a dma_fence currently need to life as long as the
dma_fence is alive. This means that the module which originally issued
a dma_fence can't unload unless all fences are freed up.
As first step to solve this issue protect the fence ops by RCU.
While it is counter intuitive to protect a constant function pointer table
by RCU it allows modules to wait for an RCU grace period before they
unload, to make sure that nobody is executing their functions any more.
This patch has not much functional change, but only adds the RCU
handling for the static checker to test.
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
v6: improve code comments
v7: improve commit message and code comments
v8: fix sparse rcu warnings
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Link: https://lore.kernel.org/r/20260219160822.1529-2-christian.koenig@amd.com
+80
-30
+49
-22
drivers/dma-buf/dma-fence.c
+49
-22
drivers/dma-buf/dma-fence.c
···
522
522
signed long
523
523
dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
524
524
{
525
+
const struct dma_fence_ops *ops;
525
526
signed long ret;
526
527
527
528
if (WARN_ON(timeout < 0))
···
534
533
535
534
dma_fence_enable_sw_signaling(fence);
536
535
537
-
if (trace_dma_fence_wait_start_enabled()) {
538
-
rcu_read_lock();
539
-
trace_dma_fence_wait_start(fence);
536
+
rcu_read_lock();
537
+
ops = rcu_dereference(fence->ops);
538
+
trace_dma_fence_wait_start(fence);
539
+
if (ops->wait) {
540
+
/*
541
+
* Implementing the wait ops is deprecated and not supported for
542
+
* issuers of fences who need their lifetime to be independent
543
+
* of their module after they signal, so it is ok to use the
544
+
* ops outside the RCU protected section.
545
+
*/
540
546
rcu_read_unlock();
541
-
}
542
-
if (fence->ops->wait)
543
-
ret = fence->ops->wait(fence, intr, timeout);
544
-
else
547
+
ret = ops->wait(fence, intr, timeout);
548
+
} else {
549
+
rcu_read_unlock();
545
550
ret = dma_fence_default_wait(fence, intr, timeout);
551
+
}
546
552
if (trace_dma_fence_wait_end_enabled()) {
547
553
rcu_read_lock();
548
554
trace_dma_fence_wait_end(fence);
···
570
562
{
571
563
struct dma_fence *fence =
572
564
container_of(kref, struct dma_fence, refcount);
565
+
const struct dma_fence_ops *ops;
573
566
574
567
rcu_read_lock();
575
568
trace_dma_fence_destroy(fence);
···
602
593
spin_unlock_irqrestore(fence->lock, flags);
603
594
}
604
595
605
-
rcu_read_unlock();
606
-
607
-
if (fence->ops->release)
608
-
fence->ops->release(fence);
596
+
ops = rcu_dereference(fence->ops);
597
+
if (ops->release)
598
+
ops->release(fence);
609
599
else
610
600
dma_fence_free(fence);
601
+
rcu_read_unlock();
611
602
}
612
603
EXPORT_SYMBOL(dma_fence_release);
613
604
···
626
617
627
618
static bool __dma_fence_enable_signaling(struct dma_fence *fence)
628
619
{
620
+
const struct dma_fence_ops *ops;
629
621
bool was_set;
630
622
631
623
lockdep_assert_held(fence->lock);
···
637
627
if (dma_fence_test_signaled_flag(fence))
638
628
return false;
639
629
640
-
if (!was_set && fence->ops->enable_signaling) {
630
+
rcu_read_lock();
631
+
ops = rcu_dereference(fence->ops);
632
+
if (!was_set && ops->enable_signaling) {
641
633
trace_dma_fence_enable_signal(fence);
642
634
643
-
if (!fence->ops->enable_signaling(fence)) {
635
+
if (!ops->enable_signaling(fence)) {
636
+
rcu_read_unlock();
644
637
dma_fence_signal_locked(fence);
645
638
return false;
646
639
}
647
640
}
641
+
rcu_read_unlock();
648
642
649
643
return true;
650
644
}
···
1021
1007
*/
1022
1008
void dma_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
1023
1009
{
1024
-
if (fence->ops->set_deadline && !dma_fence_is_signaled(fence))
1025
-
fence->ops->set_deadline(fence, deadline);
1010
+
const struct dma_fence_ops *ops;
1011
+
1012
+
rcu_read_lock();
1013
+
ops = rcu_dereference(fence->ops);
1014
+
if (ops->set_deadline && !dma_fence_is_signaled(fence))
1015
+
ops->set_deadline(fence, deadline);
1016
+
rcu_read_unlock();
1026
1017
}
1027
1018
EXPORT_SYMBOL(dma_fence_set_deadline);
1028
1019
···
1068
1049
BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
1069
1050
1070
1051
kref_init(&fence->refcount);
1071
-
fence->ops = ops;
1052
+
/*
1053
+
* While it is counter intuitive to protect a constant function pointer
1054
+
* table by RCU it allows modules to wait for an RCU grace period
1055
+
* before they unload, to make sure that nobody is executing their
1056
+
* functions any more.
1057
+
*/
1058
+
RCU_INIT_POINTER(fence->ops, ops);
1072
1059
INIT_LIST_HEAD(&fence->cb_list);
1073
1060
fence->lock = lock;
1074
1061
fence->context = context;
···
1154
1129
*/
1155
1130
const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
1156
1131
{
1157
-
RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
1158
-
"RCU protection is required for safe access to returned string");
1132
+
const struct dma_fence_ops *ops;
1159
1133
1134
+
/* RCU protection is required for safe access to returned string */
1135
+
ops = rcu_dereference(fence->ops);
1160
1136
if (!dma_fence_test_signaled_flag(fence))
1161
-
return (const char __rcu *)fence->ops->get_driver_name(fence);
1137
+
return (const char __rcu *)ops->get_driver_name(fence);
1162
1138
else
1163
1139
return (const char __rcu *)"detached-driver";
1164
1140
}
···
1187
1161
*/
1188
1162
const char __rcu *dma_fence_timeline_name(struct dma_fence *fence)
1189
1163
{
1190
-
RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
1191
-
"RCU protection is required for safe access to returned string");
1164
+
const struct dma_fence_ops *ops;
1192
1165
1166
+
/* RCU protection is required for safe access to returned string */
1167
+
ops = rcu_dereference(fence->ops);
1193
1168
if (!dma_fence_test_signaled_flag(fence))
1194
-
return (const char __rcu *)fence->ops->get_driver_name(fence);
1169
+
return (const char __rcu *)ops->get_driver_name(fence);
1195
1170
else
1196
1171
return (const char __rcu *)"signaled-timeline";
1197
1172
}
+1
-1
drivers/gpu/drm/drm_crtc.c
+1
-1
drivers/gpu/drm/drm_crtc.c
+2
-2
drivers/gpu/drm/scheduler/sched_fence.c
+2
-2
drivers/gpu/drm/scheduler/sched_fence.c
···
195
195
196
196
struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f)
197
197
{
198
-
if (f->ops == &drm_sched_fence_ops_scheduled)
198
+
if (rcu_access_pointer(f->ops) == &drm_sched_fence_ops_scheduled)
199
199
return container_of(f, struct drm_sched_fence, scheduled);
200
200
201
-
if (f->ops == &drm_sched_fence_ops_finished)
201
+
if (rcu_access_pointer(f->ops) == &drm_sched_fence_ops_finished)
202
202
return container_of(f, struct drm_sched_fence, finished);
203
203
204
204
return NULL;
+28
-5
include/linux/dma-fence.h
+28
-5
include/linux/dma-fence.h
···
67
67
*/
68
68
struct dma_fence {
69
69
spinlock_t *lock;
70
-
const struct dma_fence_ops *ops;
70
+
const struct dma_fence_ops __rcu *ops;
71
71
/*
72
72
* We clear the callback list on kref_put so that by the time we
73
73
* release the fence it is unused. No one should be adding to the
···
220
220
* timed out. Can also return other error values on custom implementations,
221
221
* which should be treated as if the fence is signaled. For example a hardware
222
222
* lockup could be reported like that.
223
+
*
224
+
* Implementing this callback prevents the fence from detaching after
225
+
* signaling and so it is necessary for the module providing the
226
+
* dma_fence_ops to stay loaded as long as the dma_fence exists.
223
227
*/
224
228
signed long (*wait)(struct dma_fence *fence,
225
229
bool intr, signed long timeout);
···
235
231
* Can be called from irq context. This callback is optional. If it is
236
232
* NULL, then dma_fence_free() is instead called as the default
237
233
* implementation.
234
+
*
235
+
* Implementing this callback prevents the fence from detaching after
236
+
* signaling and so it is necessary for the module providing the
237
+
* dma_fence_ops to stay loaded as long as the dma_fence exists.
238
+
*
239
+
* If the callback is implemented the memory backing the dma_fence
240
+
* object must be freed RCU safe.
238
241
*/
239
242
void (*release)(struct dma_fence *fence);
240
243
···
465
454
static inline bool
466
455
dma_fence_is_signaled_locked(struct dma_fence *fence)
467
456
{
457
+
const struct dma_fence_ops *ops;
458
+
468
459
if (dma_fence_test_signaled_flag(fence))
469
460
return true;
470
461
471
-
if (fence->ops->signaled && fence->ops->signaled(fence)) {
462
+
rcu_read_lock();
463
+
ops = rcu_dereference(fence->ops);
464
+
if (ops->signaled && ops->signaled(fence)) {
465
+
rcu_read_unlock();
472
466
dma_fence_signal_locked(fence);
473
467
return true;
474
468
}
469
+
rcu_read_unlock();
475
470
476
471
return false;
477
472
}
···
501
484
static inline bool
502
485
dma_fence_is_signaled(struct dma_fence *fence)
503
486
{
487
+
const struct dma_fence_ops *ops;
488
+
504
489
if (dma_fence_test_signaled_flag(fence))
505
490
return true;
506
491
507
-
if (fence->ops->signaled && fence->ops->signaled(fence)) {
492
+
rcu_read_lock();
493
+
ops = rcu_dereference(fence->ops);
494
+
if (ops->signaled && ops->signaled(fence)) {
495
+
rcu_read_unlock();
508
496
dma_fence_signal(fence);
509
497
return true;
510
498
}
499
+
rcu_read_unlock();
511
500
512
501
return false;
513
502
}
···
718
695
*/
719
696
static inline bool dma_fence_is_array(struct dma_fence *fence)
720
697
{
721
-
return fence->ops == &dma_fence_array_ops;
698
+
return rcu_access_pointer(fence->ops) == &dma_fence_array_ops;
722
699
}
723
700
724
701
/**
···
729
706
*/
730
707
static inline bool dma_fence_is_chain(struct dma_fence *fence)
731
708
{
732
-
return fence->ops == &dma_fence_chain_ops;
709
+
return rcu_access_pointer(fence->ops) == &dma_fence_chain_ops;
733
710
}
734
711
735
712
/**