| Message ID | 20250509153352.7187-4-tvrtko.ursulin@igalia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Some (drm_sched_|dma_)fence lifetime issues | expand |
On 09/05/2025 16:33, Tvrtko Ursulin wrote: > With the goal of reducing the need for drivers to touch (and dereference) > fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to > the fence->flags. > > Drivers which were setting this flag are changed to use new > dma_fence_init64() instead of dma_fence_init(). Note that due rebase slip up I lost the hunk which actually removes the bool from the ops struct. Fixed locally. Regards, Tvrtko > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > --- > drivers/dma-buf/dma-fence-chain.c | 5 ++--- > drivers/dma-buf/dma-fence.c | 9 +++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c | 5 ++--- > include/linux/dma-fence.h | 6 +++++- > 4 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > index 90424f23fd73..a8a90acf4f34 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence, > } > > const struct dma_fence_ops dma_fence_chain_ops = { > - .use_64bit_seqno = true, > .get_driver_name = dma_fence_chain_get_driver_name, > .get_timeline_name = dma_fence_chain_get_timeline_name, > .enable_signaling = dma_fence_chain_enable_signaling, > @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, > seqno = max(prev->seqno, seqno); > } > > - dma_fence_init(&chain->base, &dma_fence_chain_ops, > - &chain->lock, context, seqno); > + dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock, > + context, seqno); > > /* > * Chaining dma_fence_chain container together is only allowed through > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index f0cdd3e99d36..33661658f684 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -1023,3 +1023,12 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, > trace_dma_fence_init(fence); > } > EXPORT_SYMBOL(dma_fence_init); > + > +void > +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > + spinlock_t *lock, u64 context, u64 seqno) > +{ > + dma_fence_init(fence, ops, lock, context, seqno); > + __set_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags); > +} > +EXPORT_SYMBOL(dma_fence_init64); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > index 51cddfa3f1e8..5d26797356a3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work) > } > > static const struct dma_fence_ops amdgpu_tlb_fence_ops = { > - .use_64bit_seqno = true, > .get_driver_name = amdgpu_tlb_fence_get_driver_name, > .get_timeline_name = amdgpu_tlb_fence_get_timeline_name > }; > @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm > INIT_WORK(&f->work, amdgpu_tlb_fence_work); > spin_lock_init(&f->lock); > > - dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock, > - vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); > + dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock, > + vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); > > /* TODO: We probably need a separate wq here */ > dma_fence_get(&f->base); > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 48b5202c531d..ac6535716dbe 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -97,6 +97,7 @@ struct dma_fence { > }; > > enum dma_fence_flag_bits { > + DMA_FENCE_FLAG_SEQNO64_BIT, > DMA_FENCE_FLAG_SIGNALED_BIT, > DMA_FENCE_FLAG_TIMESTAMP_BIT, > DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, > @@ -262,6 +263,9 @@ struct dma_fence_ops { > void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, > spinlock_t *lock, u64 context, u64 seqno); > > +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > + spinlock_t *lock, u64 context, u64 seqno); > + > void dma_fence_release(struct kref *kref); > void dma_fence_free(struct dma_fence *fence); > void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); > @@ -454,7 +458,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) > * 32bit sequence numbers. Use a 64bit compare when the driver says to > * do so. > */ > - if (fence->ops->use_64bit_seqno) > + if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) > return f1 > f2; > > return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
On 5/9/25 17:33, Tvrtko Ursulin wrote: > With the goal of reducing the need for drivers to touch (and dereference) > fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to > the fence->flags. > > Drivers which were setting this flag are changed to use new > dma_fence_init64() instead of dma_fence_init(). > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> My idea was to copy the flag from the ops during init, but that should work as well. Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-fence-chain.c | 5 ++--- > drivers/dma-buf/dma-fence.c | 9 +++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c | 5 ++--- > include/linux/dma-fence.h | 6 +++++- > 4 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > index 90424f23fd73..a8a90acf4f34 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence, > } > > const struct dma_fence_ops dma_fence_chain_ops = { > - .use_64bit_seqno = true, > .get_driver_name = dma_fence_chain_get_driver_name, > .get_timeline_name = dma_fence_chain_get_timeline_name, > .enable_signaling = dma_fence_chain_enable_signaling, > @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, > seqno = max(prev->seqno, seqno); > } > > - dma_fence_init(&chain->base, &dma_fence_chain_ops, > - &chain->lock, context, seqno); > + dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock, > + context, seqno); > > /* > * Chaining dma_fence_chain container together is only allowed through > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index f0cdd3e99d36..33661658f684 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -1023,3 +1023,12 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, > trace_dma_fence_init(fence); > } > EXPORT_SYMBOL(dma_fence_init); > + > +void > +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > + spinlock_t *lock, u64 context, u64 seqno) > +{ > + dma_fence_init(fence, ops, lock, context, seqno); > + __set_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags); > +} > +EXPORT_SYMBOL(dma_fence_init64); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > index 51cddfa3f1e8..5d26797356a3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c > @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work) > } > > static const struct dma_fence_ops amdgpu_tlb_fence_ops = { > - .use_64bit_seqno = true, > .get_driver_name = amdgpu_tlb_fence_get_driver_name, > .get_timeline_name = amdgpu_tlb_fence_get_timeline_name > }; > @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm > INIT_WORK(&f->work, amdgpu_tlb_fence_work); > spin_lock_init(&f->lock); > > - dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock, > - vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); > + dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock, > + vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); > > /* TODO: We probably need a separate wq here */ > dma_fence_get(&f->base); > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 48b5202c531d..ac6535716dbe 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -97,6 +97,7 @@ struct dma_fence { > }; > > enum dma_fence_flag_bits { > + DMA_FENCE_FLAG_SEQNO64_BIT, > DMA_FENCE_FLAG_SIGNALED_BIT, > DMA_FENCE_FLAG_TIMESTAMP_BIT, > DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, > @@ -262,6 +263,9 @@ struct dma_fence_ops { > void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, > spinlock_t *lock, u64 context, u64 seqno); > > +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > + spinlock_t *lock, u64 context, u64 seqno); > + > void dma_fence_release(struct kref *kref); > void dma_fence_free(struct dma_fence *fence); > void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); > @@ -454,7 +458,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) > * 32bit sequence numbers. Use a 64bit compare when the driver says to > * do so. > */ > - if (fence->ops->use_64bit_seqno) > + if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) > return f1 > f2; > > return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 90424f23fd73..a8a90acf4f34 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence, } const struct dma_fence_ops dma_fence_chain_ops = { - .use_64bit_seqno = true, .get_driver_name = dma_fence_chain_get_driver_name, .get_timeline_name = dma_fence_chain_get_timeline_name, .enable_signaling = dma_fence_chain_enable_signaling, @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, seqno = max(prev->seqno, seqno); } - dma_fence_init(&chain->base, &dma_fence_chain_ops, - &chain->lock, context, seqno); + dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock, + context, seqno); /* * Chaining dma_fence_chain container together is only allowed through diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index f0cdd3e99d36..33661658f684 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1023,3 +1023,12 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, trace_dma_fence_init(fence); } EXPORT_SYMBOL(dma_fence_init); + +void +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, + spinlock_t *lock, u64 context, u64 seqno) +{ + dma_fence_init(fence, ops, lock, context, seqno); + __set_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags); +} +EXPORT_SYMBOL(dma_fence_init64); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c index 51cddfa3f1e8..5d26797356a3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work) } static const struct dma_fence_ops amdgpu_tlb_fence_ops = { - .use_64bit_seqno = true, .get_driver_name = amdgpu_tlb_fence_get_driver_name, .get_timeline_name = amdgpu_tlb_fence_get_timeline_name }; @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm INIT_WORK(&f->work, amdgpu_tlb_fence_work); spin_lock_init(&f->lock); - dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock, - vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); + dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock, + vm->tlb_fence_context, atomic64_read(&vm->tlb_seq)); /* TODO: We probably need a separate wq here */ dma_fence_get(&f->base); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 48b5202c531d..ac6535716dbe 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -97,6 +97,7 @@ struct dma_fence { }; enum dma_fence_flag_bits { + DMA_FENCE_FLAG_SEQNO64_BIT, DMA_FENCE_FLAG_SIGNALED_BIT, DMA_FENCE_FLAG_TIMESTAMP_BIT, DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, @@ -262,6 +263,9 @@ struct dma_fence_ops { void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno); +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, + spinlock_t *lock, u64 context, u64 seqno); + void dma_fence_release(struct kref *kref); void dma_fence_free(struct dma_fence *fence); void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); @@ -454,7 +458,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2) * 32bit sequence numbers. Use a 64bit compare when the driver says to * do so. */ - if (fence->ops->use_64bit_seqno) + if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags)) return f1 > f2; return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
With the goal of reducing the need for drivers to touch (and dereference) fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to the fence->flags. Drivers which were setting this flag are changed to use new dma_fence_init64() instead of dma_fence_init(). Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> --- drivers/dma-buf/dma-fence-chain.c | 5 ++--- drivers/dma-buf/dma-fence.c | 9 +++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c | 5 ++--- include/linux/dma-fence.h | 6 +++++- 4 files changed, 18 insertions(+), 7 deletions(-)