Hi,
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the write side critical section.
There is no built-in debugging mechanism to verify that the lock used for writer serialization is held and preemption is disabled. Some usage sites like dma-buf have explicit lockdep checks for the writer-side lock, but this covers only a small portion of the sequence counter usage in the kernel.
Add new sequence counter types which allows to associate a lock to the sequence counter at initialization time. The seqcount API functions are extended to provide appropriate lockdep assertions depending on the seqcount/lock type.
For sequence counters with associated locks that do not implicitly disable preemption, preemption protection is enforced in the sequence counter write side functions. This removes the need to explicitly add preempt_disable/enable() around the write side critical sections: the write_begin/end() functions for these new sequence counter types automatically do this.
Extend the lockdep API with a macro asserting that preemption is disabled. Use it to verify that preemption is disabled for all sequence counters write side critical sections.
If lockdep is disabled, these lock associations and non-preemptibility checks are compiled out and have neither storage size nor runtime overhead. If lockdep is enabled, a pointer to the lock is stored in the seqcount and the write side API functions enable lockdep assertions.
The following seqcount types with associated locks are introduced:
seqcount_spinlock_t seqcount_raw_spinlock_t seqcount_rwlock_t seqcount_mutex_t seqcount_ww_mutex_t
This lock association is not only useful for debugging purposes, it also provides a mechanism for PREEMPT_RT to prevent writer starvation. On RT kernels spinlocks and rwlocks are substituted with sleeping locks and the code sections protected by these locks become preemptible, which has the same problem as write side critical section with preemption enabled on a non-RT kernel. RT utilizes this association by storing the provided lock pointer and in case that a reader sees an active writer (seqcount is odd), it does not spin, but blocks on the associated lock similar to read_seqbegin_or_lock().
By using the lockdep debugging mechanisms added in this patch series, a number of erroneous seqcount call-sites were discovered across the kernel. The fixes are included at the beginning of the series.
Thanks,
8<--------------
Ahmed S. Darwish (25): net: core: device_rename: Use rwsem instead of a seqcount mm/swap: Don't abuse the seqcount latching API net: phy: fixed_phy: Remove unused seqcount block: nr_sects_write(): Disable preemption on seqcount write u64_stats: Document writer non-preemptibility requirement dma-buf: Remove custom seqcount lockdep class key lockdep: Add preemption disabled assertion API seqlock: lockdep assert non-preemptibility on seqcount_t write Documentation: locking: Describe seqlock design and usage seqlock: Add RST directives to kernel-doc code samples and notes seqlock: Add missing kernel-doc annotations seqlock: Extend seqcount API with associated locks dma-buf: Use sequence counter with associated wound/wait mutex sched: tasks: Use sequence counter with associated spinlock netfilter: conntrack: Use sequence counter with associated spinlock netfilter: nft_set_rbtree: Use sequence counter with associated rwlock xfrm: policy: Use sequence counters with associated lock timekeeping: Use sequence counter with associated raw spinlock vfs: Use sequence counter with associated spinlock raid5: Use sequence counter with associated spinlock iocost: Use sequence counter with associated spinlock NFSv4: Use sequence counter with associated spinlock userfaultfd: Use sequence counter with associated spinlock kvm/eventfd: Use sequence counter with associated spinlock hrtimer: Use sequence counter with associated raw spinlock
Documentation/locking/index.rst | 1 + Documentation/locking/seqlock.rst | 239 +++++ MAINTAINERS | 2 +- block/blk-iocost.c | 5 +- block/blk.h | 2 + drivers/dma-buf/dma-resv.c | 15 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 - drivers/md/raid5.c | 2 +- drivers/md/raid5.h | 2 +- drivers/net/phy/fixed_phy.c | 25 +- fs/dcache.c | 2 +- fs/fs_struct.c | 4 +- fs/nfs/nfs4_fs.h | 2 +- fs/nfs/nfs4state.c | 2 +- fs/userfaultfd.c | 4 +- include/linux/dcache.h | 2 +- include/linux/dma-resv.h | 4 +- include/linux/fs_struct.h | 2 +- include/linux/hrtimer.h | 2 +- include/linux/kvm_irqfd.h | 2 +- include/linux/lockdep.h | 9 + include/linux/sched.h | 2 +- include/linux/seqlock.h | 882 +++++++++++++++--- include/linux/seqlock_types_internal.h | 187 ++++ include/linux/u64_stats_sync.h | 38 +- include/net/netfilter/nf_conntrack.h | 2 +- init/init_task.c | 3 +- kernel/fork.c | 2 +- kernel/locking/lockdep.c | 15 + kernel/time/hrtimer.c | 13 +- kernel/time/timekeeping.c | 19 +- lib/Kconfig.debug | 1 + mm/swap.c | 57 +- net/core/dev.c | 30 +- net/netfilter/nf_conntrack_core.c | 5 +- net/netfilter/nft_set_rbtree.c | 4 +- net/xfrm/xfrm_policy.c | 10 +- virt/kvm/eventfd.c | 2 +- 38 files changed, 1325 insertions(+), 277 deletions(-) create mode 100644 Documentation/locking/seqlock.rst create mode 100644 include/linux/seqlock_types_internal.h
base-commit: 2ef96a5bb12be62ef75b5828c0aab838ebb29cb8 -- 2.20.1
Commit 3c3b177a9369 ("reservation: add support for read-only access using rcu") introduced a sequence counter to manage updates to reservations. Back then, the reservation object initializer reservation_object_init() was always inlined.
Having the sequence counter initialization inlined meant that each of the call sites would have a different lockdep class key, which would've broken lockdep's deadlock detection. The aforementioned commit thus introduced, and exported, a custom seqcount lockdep class key and name.
The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...") transformed the reservation object initializer to a normal non-inlined C function. seqcount_init(), which automatically defines the seqcount lockdep class key and must be called non-inlined, can now be safely used.
Remove the seqcount custom lockdep class key, name, and export. Use seqcount_init() inside the dma reservation object initializer.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de Reviewed-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/dma-buf/dma-resv.c | 9 +-------- include/linux/dma-resv.h | 2 -- 2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 4264e64788c4..590ce7ad60a0 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -50,12 +50,6 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class);
-struct lock_class_key reservation_seqcount_class; -EXPORT_SYMBOL(reservation_seqcount_class); - -const char reservation_seqcount_string[] = "reservation_seqcount"; -EXPORT_SYMBOL(reservation_seqcount_string); - /** * dma_resv_list_alloc - allocate fence list * @shared_max: number of fences we need space for @@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); + seqcount_init(&obj->seq);
- __seqcount_init(&obj->seq, reservation_seqcount_string, - &reservation_seqcount_class); RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); } diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index ee50d10f052b..a6538ae7d93f 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -46,8 +46,6 @@ #include <linux/rcupdate.h>
extern struct ww_class reservation_ww_class; -extern struct lock_class_key reservation_seqcount_class; -extern const char reservation_seqcount_string[];
/** * struct dma_resv_list - a list of shared fences
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
- It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
- It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de --- drivers/dma-buf/dma-resv.c | 8 +------- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 -- include/linux/dma-resv.h | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 590ce7ad60a0..3aba2b2bfc48 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -128,7 +128,7 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); - seqcount_init(&obj->seq); + seqcount_ww_mutex_init(&obj->seq, &obj->lock);
RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); @@ -259,7 +259,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) fobj = dma_resv_get_list(obj); count = fobj->shared_count;
- preempt_disable(); write_seqcount_begin(&obj->seq);
for (i = 0; i < count; ++i) { @@ -281,7 +280,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) smp_store_mb(fobj->shared_count, count);
write_seqcount_end(&obj->seq); - preempt_enable(); dma_fence_put(old); } EXPORT_SYMBOL(dma_resv_add_shared_fence); @@ -308,14 +306,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) if (fence) dma_fence_get(fence);
- preempt_disable(); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); if (old) old->shared_count = 0; write_seqcount_end(&obj->seq); - preempt_enable();
/* inplace update, no shared fences */ while (i--) @@ -393,13 +389,11 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) src_list = dma_resv_get_list(dst); old = dma_resv_get_excl(dst);
- preempt_disable(); write_seqcount_begin(&dst->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(dst->fence_excl, new); RCU_INIT_POINTER(dst->fence, dst_list); write_seqcount_end(&dst->seq); - preempt_enable();
dma_resv_list_free(src_list); dma_fence_put(old); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 9dff792c9290..87fd32aae8f9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -258,11 +258,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, new->shared_count = k;
/* Install the new fence list, seqcount provides the barriers */ - preempt_disable(); write_seqcount_begin(&resv->seq); RCU_INIT_POINTER(resv->fence, new); write_seqcount_end(&resv->seq); - preempt_enable();
/* Drop the references to the removed fences or move them to ef_list */ for (i = j, k = 0; i < old->shared_count; ++i) { diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index a6538ae7d93f..d44a77e8a7e3 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -69,7 +69,7 @@ struct dma_resv_list { */ struct dma_resv { struct ww_mutex lock; - seqcount_t seq; + seqcount_ww_mutex_t seq;
struct dma_fence __rcu *fence_excl; struct dma_resv_list __rcu *fence;
Am 19.05.20 um 23:45 schrieb Ahmed S. Darwish:
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Mhm, is the dma_resv object the only user of this new seqcount_ww_mutex variant ?
If yes we are trying to get rid of this sequence counter for quite some time, so I would rather invest the additional time to finish this.
Regards, Christian.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de
drivers/dma-buf/dma-resv.c | 8 +------- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 -- include/linux/dma-resv.h | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 590ce7ad60a0..3aba2b2bfc48 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -128,7 +128,7 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class);
- seqcount_init(&obj->seq);
seqcount_ww_mutex_init(&obj->seq, &obj->lock);
RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL);
@@ -259,7 +259,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) fobj = dma_resv_get_list(obj); count = fobj->shared_count;
preempt_disable(); write_seqcount_begin(&obj->seq);
for (i = 0; i < count; ++i) {
@@ -281,7 +280,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) smp_store_mb(fobj->shared_count, count);
write_seqcount_end(&obj->seq);
- preempt_enable(); dma_fence_put(old); } EXPORT_SYMBOL(dma_resv_add_shared_fence);
@@ -308,14 +306,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) if (fence) dma_fence_get(fence);
preempt_disable(); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); if (old) old->shared_count = 0; write_seqcount_end(&obj->seq);
preempt_enable();
/* inplace update, no shared fences */ while (i--)
@@ -393,13 +389,11 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) src_list = dma_resv_get_list(dst); old = dma_resv_get_excl(dst);
preempt_disable(); write_seqcount_begin(&dst->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(dst->fence_excl, new); RCU_INIT_POINTER(dst->fence, dst_list); write_seqcount_end(&dst->seq);
preempt_enable();
dma_resv_list_free(src_list); dma_fence_put(old);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 9dff792c9290..87fd32aae8f9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -258,11 +258,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, new->shared_count = k;
/* Install the new fence list, seqcount provides the barriers */
preempt_disable(); write_seqcount_begin(&resv->seq); RCU_INIT_POINTER(resv->fence, new); write_seqcount_end(&resv->seq);
preempt_enable();
/* Drop the references to the removed fences or move them to ef_list */ for (i = j, k = 0; i < old->shared_count; ++i) {
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index a6538ae7d93f..d44a77e8a7e3 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -69,7 +69,7 @@ struct dma_resv_list { */ struct dma_resv { struct ww_mutex lock;
- seqcount_t seq;
seqcount_ww_mutex_t seq;
struct dma_fence __rcu *fence_excl; struct dma_resv_list __rcu *fence;
On Wed, May 20, 2020, Christian König wrote:
Am 19.05.20 um 23:45 schrieb Ahmed S. Darwish:
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Mhm, is the dma_resv object the only user of this new seqcount_ww_mutex variant ?
If yes we are trying to get rid of this sequence counter for quite some time, so I would rather invest the additional time to finish this.
In this patch series, each extra "seqcount with associated lock" data type costs us, exactly:
- 1 typedef definition, seqcount_ww_mutex_t - 1 static initializer, SEQCNT_WW_MUTEX_ZERO() - 1 runtime initializer, seqcount_ww_mutex_init()
Definitions for the typedef and the 2 initializers above are template-code one liners.
The logic which automatically disables preemption upon entering a seqcount_ww_mutex_t write side critical section is also already shared with seqcount_mutex_t and any future, preemptible, associated lock.
So, yes, dma-resv is the only user of seqcount_ww_mutex.
But even in that case, given the one liner template code nature of seqcount_ww_mutex_t logic, it does not make sense to block the dma_resv and amdgpu change until at some point in the future the sequence counter is completely removed.
**If and when** the sequence counter gets removed, please just remove the seqcount_ww_mutex_t data type with it. It will be extremely simple.
Regards, Christian.
Thanks,
-- Ahmed S. Darwish Linutronix GmbH
Am 21.05.20 um 02:09 schrieb Ahmed S. Darwish:
On Wed, May 20, 2020, Christian König wrote:
Am 19.05.20 um 23:45 schrieb Ahmed S. Darwish:
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
- It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized. - It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Mhm, is the dma_resv object the only user of this new seqcount_ww_mutex variant ?
If yes we are trying to get rid of this sequence counter for quite some time, so I would rather invest the additional time to finish this.
In this patch series, each extra "seqcount with associated lock" data type costs us, exactly:
- 1 typedef definition, seqcount_ww_mutex_t
- 1 static initializer, SEQCNT_WW_MUTEX_ZERO()
- 1 runtime initializer, seqcount_ww_mutex_init()
Definitions for the typedef and the 2 initializers above are template-code one liners.
In this case I'm perfectly fine with this.
The logic which automatically disables preemption upon entering a seqcount_ww_mutex_t write side critical section is also already shared with seqcount_mutex_t and any future, preemptible, associated lock.
So, yes, dma-resv is the only user of seqcount_ww_mutex.
But even in that case, given the one liner template code nature of seqcount_ww_mutex_t logic, it does not make sense to block the dma_resv and amdgpu change until at some point in the future the sequence counter is completely removed.
**If and when** the sequence counter gets removed, please just remove the seqcount_ww_mutex_t data type with it. It will be extremely simple.
Completely agree, just wanted to prevent that we now add a lot of code which gets removed again ~3 month from now.
Regards, Christian.
Regards, Christian.
Thanks,
-- Ahmed S. Darwish Linutronix GmbH
Hi,
This is v2 of the seqlock patch series:
[PATCH v1 00/25] seqlock: Extend seqcount API with associated locks https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
Patches 1=>3 of this v2 series add documentation for the existing seqlock.h datatypes and APIs. Hopefully they can hit v5.8 -rc2 or -rc3.
Changelog-v2 ============
1. Drop, for now, the seqlock v1 patches #7 and #8. These patches added lockdep non-preemptibility checks to seqcount_t write paths, but they now depend on on-going work by Peter:
[PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
[PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
Once Peter's work get merged, I'll send the non-preemptibility checks as a separate series.
2. Drop the v1 seqcount_t call-sites bugfixes. I've already posted them in an isolated series. They got merged into their respective trees, and will hit v5.8-rc1 soon:
[PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes https://lore.kernel.org/lkml/20200603144949.1122421-1-a.darwish@linutronix.d...
3. Patch #1: Add a small paragraph explaining that seqcount_t/seqlock_t cannot be used if the protected data contains pointers. A similar paragraph already existed in seqlock.h, but got mistakenly dropped.
4. Patch #2: Don't add RST directives inside kernel-doc comments. Peter doesn't like them :) I've kept the indentation though, and found a minimal way for Sphinx to properly render these code samples without too much disruption.
5. Patch #3: Brush up the introduced kernel-doc comments. Make them more consistent overall, and more concise.
Thanks,
8<--------------
Ahmed S. Darwish (18): Documentation: locking: Describe seqlock design and usage seqlock: Properly format kernel-doc code samples seqlock: Add missing kernel-doc annotations seqlock: Extend seqcount API with associated locks dma-buf: Remove custom seqcount lockdep class key dma-buf: Use sequence counter with associated wound/wait mutex sched: tasks: Use sequence counter with associated spinlock netfilter: conntrack: Use sequence counter with associated spinlock netfilter: nft_set_rbtree: Use sequence counter with associated rwlock xfrm: policy: Use sequence counters with associated lock timekeeping: Use sequence counter with associated raw spinlock vfs: Use sequence counter with associated spinlock raid5: Use sequence counter with associated spinlock iocost: Use sequence counter with associated spinlock NFSv4: Use sequence counter with associated spinlock userfaultfd: Use sequence counter with associated spinlock kvm/eventfd: Use sequence counter with associated spinlock hrtimer: Use sequence counter with associated raw spinlock
Documentation/locking/index.rst | 1 + Documentation/locking/seqlock.rst | 242 +++++ MAINTAINERS | 2 +- block/blk-iocost.c | 5 +- drivers/dma-buf/dma-resv.c | 15 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 - drivers/md/raid5.c | 2 +- drivers/md/raid5.h | 2 +- fs/dcache.c | 2 +- fs/fs_struct.c | 4 +- fs/nfs/nfs4_fs.h | 2 +- fs/nfs/nfs4state.c | 2 +- fs/userfaultfd.c | 4 +- include/linux/dcache.h | 2 +- include/linux/dma-resv.h | 4 +- include/linux/fs_struct.h | 2 +- include/linux/hrtimer.h | 2 +- include/linux/kvm_irqfd.h | 2 +- include/linux/sched.h | 2 +- include/linux/seqlock.h | 855 ++++++++++++++---- include/linux/seqlock_types_internal.h | 187 ++++ include/net/netfilter/nf_conntrack.h | 2 +- init/init_task.c | 3 +- kernel/fork.c | 2 +- kernel/time/hrtimer.c | 13 +- kernel/time/timekeeping.c | 19 +- net/netfilter/nf_conntrack_core.c | 5 +- net/netfilter/nft_set_rbtree.c | 4 +- net/xfrm/xfrm_policy.c | 10 +- virt/kvm/eventfd.c | 2 +- 30 files changed, 1175 insertions(+), 226 deletions(-) create mode 100644 Documentation/locking/seqlock.rst create mode 100644 include/linux/seqlock_types_internal.h
base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162 -- 2.20.1
Commit 3c3b177a9369 ("reservation: add support for read-only access using rcu") introduced a sequence counter to manage updates to reservations. Back then, the reservation object initializer reservation_object_init() was always inlined.
Having the sequence counter initialization inlined meant that each of the call sites would have a different lockdep class key, which would've broken lockdep's deadlock detection. The aforementioned commit thus introduced, and exported, a custom seqcount lockdep class key and name.
The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...") transformed the reservation object initializer to a normal non-inlined C function. seqcount_init(), which automatically defines the seqcount lockdep class key and must be called non-inlined, can now be safely used.
Remove the seqcount custom lockdep class key, name, and export. Use seqcount_init() inside the dma reservation object initializer.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de Reviewed-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/dma-buf/dma-resv.c | 9 +-------- include/linux/dma-resv.h | 2 -- 2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 4264e64788c4..590ce7ad60a0 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -50,12 +50,6 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class);
-struct lock_class_key reservation_seqcount_class; -EXPORT_SYMBOL(reservation_seqcount_class); - -const char reservation_seqcount_string[] = "reservation_seqcount"; -EXPORT_SYMBOL(reservation_seqcount_string); - /** * dma_resv_list_alloc - allocate fence list * @shared_max: number of fences we need space for @@ -134,9 +128,8 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); + seqcount_init(&obj->seq);
- __seqcount_init(&obj->seq, reservation_seqcount_string, - &reservation_seqcount_class); RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); } diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index ee50d10f052b..a6538ae7d93f 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -46,8 +46,6 @@ #include <linux/rcupdate.h>
extern struct ww_class reservation_ww_class; -extern struct lock_class_key reservation_seqcount_class; -extern const char reservation_seqcount_string[];
/** * struct dma_resv_list - a list of shared fences
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
- It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
- It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de --- drivers/dma-buf/dma-resv.c | 8 +------- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 -- include/linux/dma-resv.h | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 590ce7ad60a0..3aba2b2bfc48 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -128,7 +128,7 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); - seqcount_init(&obj->seq); + seqcount_ww_mutex_init(&obj->seq, &obj->lock);
RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); @@ -259,7 +259,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) fobj = dma_resv_get_list(obj); count = fobj->shared_count;
- preempt_disable(); write_seqcount_begin(&obj->seq);
for (i = 0; i < count; ++i) { @@ -281,7 +280,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) smp_store_mb(fobj->shared_count, count);
write_seqcount_end(&obj->seq); - preempt_enable(); dma_fence_put(old); } EXPORT_SYMBOL(dma_resv_add_shared_fence); @@ -308,14 +306,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) if (fence) dma_fence_get(fence);
- preempt_disable(); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); if (old) old->shared_count = 0; write_seqcount_end(&obj->seq); - preempt_enable();
/* inplace update, no shared fences */ while (i--) @@ -393,13 +389,11 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) src_list = dma_resv_get_list(dst); old = dma_resv_get_excl(dst);
- preempt_disable(); write_seqcount_begin(&dst->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(dst->fence_excl, new); RCU_INIT_POINTER(dst->fence, dst_list); write_seqcount_end(&dst->seq); - preempt_enable();
dma_resv_list_free(src_list); dma_fence_put(old); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 6a5b91d23fd9..c71c0bb6ce26 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -258,11 +258,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, new->shared_count = k;
/* Install the new fence list, seqcount provides the barriers */ - preempt_disable(); write_seqcount_begin(&resv->seq); RCU_INIT_POINTER(resv->fence, new); write_seqcount_end(&resv->seq); - preempt_enable();
/* Drop the references to the removed fences or move them to ef_list */ for (i = j, k = 0; i < old->shared_count; ++i) { diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index a6538ae7d93f..d44a77e8a7e3 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -69,7 +69,7 @@ struct dma_resv_list { */ struct dma_resv { struct ww_mutex lock; - seqcount_t seq; + seqcount_ww_mutex_t seq;
struct dma_fence __rcu *fence_excl; struct dma_resv_list __rcu *fence;
On Mon, Jun 08, 2020 at 02:57:17AM +0200, Ahmed S. Darwish wrote:
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de
I'm not seeing the patch that adds the seqcount ww_mutex glue and not quite motivated enough to grab it from lore, so someone else needs to check the details. Just
Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
for merging through whatever tree/branch makes sense from me. -Daniel
drivers/dma-buf/dma-resv.c | 8 +------- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 -- include/linux/dma-resv.h | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 590ce7ad60a0..3aba2b2bfc48 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -128,7 +128,7 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class);
- seqcount_init(&obj->seq);
seqcount_ww_mutex_init(&obj->seq, &obj->lock);
RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL);
@@ -259,7 +259,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) fobj = dma_resv_get_list(obj); count = fobj->shared_count;
preempt_disable(); write_seqcount_begin(&obj->seq);
for (i = 0; i < count; ++i) {
@@ -281,7 +280,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) smp_store_mb(fobj->shared_count, count);
write_seqcount_end(&obj->seq);
- preempt_enable(); dma_fence_put(old);
} EXPORT_SYMBOL(dma_resv_add_shared_fence); @@ -308,14 +306,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) if (fence) dma_fence_get(fence);
preempt_disable(); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); if (old) old->shared_count = 0; write_seqcount_end(&obj->seq);
preempt_enable();
/* inplace update, no shared fences */ while (i--)
@@ -393,13 +389,11 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) src_list = dma_resv_get_list(dst); old = dma_resv_get_excl(dst);
preempt_disable(); write_seqcount_begin(&dst->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(dst->fence_excl, new); RCU_INIT_POINTER(dst->fence, dst_list); write_seqcount_end(&dst->seq);
preempt_enable();
dma_resv_list_free(src_list); dma_fence_put(old);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 6a5b91d23fd9..c71c0bb6ce26 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -258,11 +258,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, new->shared_count = k;
/* Install the new fence list, seqcount provides the barriers */
preempt_disable(); write_seqcount_begin(&resv->seq); RCU_INIT_POINTER(resv->fence, new); write_seqcount_end(&resv->seq);
preempt_enable();
/* Drop the references to the removed fences or move them to ef_list */ for (i = j, k = 0; i < old->shared_count; ++i) {
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index a6538ae7d93f..d44a77e8a7e3 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -69,7 +69,7 @@ struct dma_resv_list { */ struct dma_resv { struct ww_mutex lock;
- seqcount_t seq;
seqcount_ww_mutex_t seq;
struct dma_fence __rcu *fence_excl; struct dma_resv_list __rcu *fence;
-- 2.20.1
Hi,
This is v3 of the seqlock patch series:
[PATCH v1 00/25] seqlock: Extend seqcount API with associated locks https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
[PATCH v2 00/18] https://lore.kernel.org/lkml/20200608005729.1874024-1-a.darwish@linutronix.d...
It's based over:
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git locking/core
to get Peter's lockdep irqstate tracking series below, which untangles mainline seqlock.h<=>sched.h 'current->' task_struct circular dependency:
https://lkml.kernel.org/r/linuxppc-dev/20200623083645.277342609@infradead.or...
Changelog-v3:
- Re-add lockdep non-preemptibility checks on seqcount_t write paths. They were removed from v2 due to the circular dependencies mentioned.
- Slight rebase over the new v5.8-rc1 KCSAN seqlock.h changes
- Collect seqcount_t call-sites acked-by tags
Thanks,
8<--------------
Ahmed S. Darwish (20): Documentation: locking: Describe seqlock design and usage seqlock: Properly format kernel-doc code samples seqlock: Add missing kernel-doc annotations lockdep: Add preemption enabled/disabled assertion APIs seqlock: lockdep assert non-preemptibility on seqcount_t write seqlock: Extend seqcount API with associated locks dma-buf: Remove custom seqcount lockdep class key dma-buf: Use sequence counter with associated wound/wait mutex sched: tasks: Use sequence counter with associated spinlock netfilter: conntrack: Use sequence counter with associated spinlock netfilter: nft_set_rbtree: Use sequence counter with associated rwlock xfrm: policy: Use sequence counters with associated lock timekeeping: Use sequence counter with associated raw spinlock vfs: Use sequence counter with associated spinlock raid5: Use sequence counter with associated spinlock iocost: Use sequence counter with associated spinlock NFSv4: Use sequence counter with associated spinlock userfaultfd: Use sequence counter with associated spinlock kvm/eventfd: Use sequence counter with associated spinlock hrtimer: Use sequence counter with associated raw spinlock
Documentation/locking/index.rst | 1 + Documentation/locking/seqlock.rst | 242 +++++ MAINTAINERS | 2 +- block/blk-iocost.c | 5 +- drivers/dma-buf/dma-resv.c | 15 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 - drivers/md/raid5.c | 2 +- drivers/md/raid5.h | 2 +- fs/dcache.c | 2 +- fs/fs_struct.c | 4 +- fs/nfs/nfs4_fs.h | 2 +- fs/nfs/nfs4state.c | 2 +- fs/userfaultfd.c | 4 +- include/linux/dcache.h | 2 +- include/linux/dma-resv.h | 4 +- include/linux/fs_struct.h | 2 +- include/linux/hrtimer.h | 2 +- include/linux/kvm_irqfd.h | 2 +- include/linux/lockdep.h | 18 + include/linux/sched.h | 2 +- include/linux/seqlock.h | 872 ++++++++++++++---- include/linux/seqlock_types_internal.h | 186 ++++ include/net/netfilter/nf_conntrack.h | 2 +- init/init_task.c | 3 +- kernel/fork.c | 2 +- kernel/time/hrtimer.c | 13 +- kernel/time/timekeeping.c | 19 +- lib/Kconfig.debug | 1 + net/netfilter/nf_conntrack_core.c | 5 +- net/netfilter/nft_set_rbtree.c | 4 +- net/xfrm/xfrm_policy.c | 10 +- virt/kvm/eventfd.c | 2 +- 32 files changed, 1211 insertions(+), 225 deletions(-) create mode 100644 Documentation/locking/seqlock.rst create mode 100644 include/linux/seqlock_types_internal.h
base-commit: 997e89fa345e9006f311cf9f9c8fd9f7d96c240f -- 2.20.1
Commit 3c3b177a9369 ("reservation: add support for read-only access using rcu") introduced a sequence counter to manage updates to reservations. Back then, the reservation object initializer reservation_object_init() was always inlined.
Having the sequence counter initialization inlined meant that each of the call sites would have a different lockdep class key, which would've broken lockdep's deadlock detection. The aforementioned commit thus introduced, and exported, a custom seqcount lockdep class key and name.
The commit 8735f16803f00 ("dma-buf: cleanup reservation_object_init...") transformed the reservation object initializer to a normal non-inlined C function. seqcount_init(), which automatically defines the seqcount lockdep class key and must be called non-inlined, can now be safely used.
Remove the seqcount custom lockdep class key, name, and export. Use seqcount_init() inside the dma reservation object initializer.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de Reviewed-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/dma-buf/dma-resv.c | 9 +-------- include/linux/dma-resv.h | 2 -- 2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index b45f8514dc82..15efa0c2dacb 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -51,12 +51,6 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class);
-struct lock_class_key reservation_seqcount_class; -EXPORT_SYMBOL(reservation_seqcount_class); - -const char reservation_seqcount_string[] = "reservation_seqcount"; -EXPORT_SYMBOL(reservation_seqcount_string); - /** * dma_resv_list_alloc - allocate fence list * @shared_max: number of fences we need space for @@ -135,9 +129,8 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); + seqcount_init(&obj->seq);
- __seqcount_init(&obj->seq, reservation_seqcount_string, - &reservation_seqcount_class); RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); } diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index ee50d10f052b..a6538ae7d93f 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -46,8 +46,6 @@ #include <linux/rcupdate.h>
extern struct ww_class reservation_ww_class; -extern struct lock_class_key reservation_seqcount_class; -extern const char reservation_seqcount_string[];
/** * struct dma_resv_list - a list of shared fences
A sequence counter write side critical section must be protected by some form of locking to serialize writers. If the serialization primitive is not disabling preemption implicitly, preemption has to be explicitly disabled before entering the sequence counter write side critical section.
The dma-buf reservation subsystem uses plain sequence counters to manage updates to reservations. Writer serialization is accomplished through a wound/wait mutex.
Acquiring a wound/wait mutex does not disable preemption, so this needs to be done manually before and after the write side critical section.
Use the newly-added seqcount_ww_mutex_t instead:
- It associates the ww_mutex with the sequence count, which enables lockdep to validate that the write side critical section is properly serialized.
- It removes the need to explicitly add preempt_disable/enable() around the write side critical section because the write_begin/end() functions for this new data type automatically do this.
If lockdep is disabled this ww_mutex lock association is compiled out and has neither storage size nor runtime overhead.
Signed-off-by: Ahmed S. Darwish a.darwish@linutronix.de Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/dma-buf/dma-resv.c | 8 +------- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 -- include/linux/dma-resv.h | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 15efa0c2dacb..a7631352a486 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -129,7 +129,7 @@ subsys_initcall(dma_resv_lockdep); void dma_resv_init(struct dma_resv *obj) { ww_mutex_init(&obj->lock, &reservation_ww_class); - seqcount_init(&obj->seq); + seqcount_ww_mutex_init(&obj->seq, &obj->lock);
RCU_INIT_POINTER(obj->fence, NULL); RCU_INIT_POINTER(obj->fence_excl, NULL); @@ -260,7 +260,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) fobj = dma_resv_get_list(obj); count = fobj->shared_count;
- preempt_disable(); write_seqcount_begin(&obj->seq);
for (i = 0; i < count; ++i) { @@ -282,7 +281,6 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) smp_store_mb(fobj->shared_count, count);
write_seqcount_end(&obj->seq); - preempt_enable(); dma_fence_put(old); } EXPORT_SYMBOL(dma_resv_add_shared_fence); @@ -309,14 +307,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) if (fence) dma_fence_get(fence);
- preempt_disable(); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); if (old) old->shared_count = 0; write_seqcount_end(&obj->seq); - preempt_enable();
/* inplace update, no shared fences */ while (i--) @@ -394,13 +390,11 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) src_list = dma_resv_get_list(dst); old = dma_resv_get_excl(dst);
- preempt_disable(); write_seqcount_begin(&dst->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(dst->fence_excl, new); RCU_INIT_POINTER(dst->fence, dst_list); write_seqcount_end(&dst->seq); - preempt_enable();
dma_resv_list_free(src_list); dma_fence_put(old); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index b91b5171270f..ff4b583cb96a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -258,11 +258,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, new->shared_count = k;
/* Install the new fence list, seqcount provides the barriers */ - preempt_disable(); write_seqcount_begin(&resv->seq); RCU_INIT_POINTER(resv->fence, new); write_seqcount_end(&resv->seq); - preempt_enable();
/* Drop the references to the removed fences or move them to ef_list */ for (i = j, k = 0; i < old->shared_count; ++i) { diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index a6538ae7d93f..d44a77e8a7e3 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -69,7 +69,7 @@ struct dma_resv_list { */ struct dma_resv { struct ww_mutex lock; - seqcount_t seq; + seqcount_ww_mutex_t seq;
struct dma_fence __rcu *fence_excl; struct dma_resv_list __rcu *fence;
dri-devel@lists.freedesktop.org
-
Ahmed S. Darwish -
Christian König -
Christian König -
Daniel Vetter