[PATCH] dma-buf/fence: make fence context 64 bit

Daniel Vetter daniel at ffwll.ch
Thu May 19 09:14:02 UTC 2016


On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> Fence contexts are created on the fly (for example) by the GPU scheduler used
> in the amdgpu driver as a result of an userspace request. Because of this
> userspace could in theory force a wrap around of the 32bit context number
> if it doesn't behave well.
> 
> Avoid this by increasing the context number to 64bits. This way even when
> userspace manages to allocate a billion contexts per second it takes more
> than 500 years for the context number to wrap around.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>

Hm, I think it'd be nice to wrap this up into a real struct and then
manage them with some idr or whatever. For debugging we might also want to
keep track of all fences on a given timeline and similar things, so
there will be a need for this in the future.

So if you go through every driver I think it's better to replace the type
with struct fence_context *context while we're at it. Makes it a notch
bigger since we need to add a little bit of error handling to all callers
of fence_context_alloc.

Volunteered? ;-)

Cheers, Daniel
> ---
>  drivers/dma-buf/fence.c                 | 8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h   | 2 +-
>  drivers/gpu/drm/nouveau/nouveau_fence.h | 3 ++-
>  drivers/gpu/drm/radeon/radeon.h         | 2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c   | 2 +-
>  drivers/staging/android/sync.h          | 3 ++-
>  include/linux/fence.h                   | 7 ++++---
>  8 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 7b05dbe..4d51f9e 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -35,7 +35,7 @@ EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>   * context or not. One device can have multiple separate contexts,
>   * and they're used if some engine can run independently of another.
>   */
> -static atomic_t fence_context_counter = ATOMIC_INIT(0);
> +static atomic64_t fence_context_counter = ATOMIC64_INIT(0);
>  
>  /**
>   * fence_context_alloc - allocate an array of fence contexts
> @@ -44,10 +44,10 @@ static atomic_t fence_context_counter = ATOMIC_INIT(0);
>   * This function will return the first index of the number of fences allocated.
>   * The fence context is used for setting fence->context to a unique number.
>   */
> -unsigned fence_context_alloc(unsigned num)
> +u64 fence_context_alloc(unsigned num)
>  {
>  	BUG_ON(!num);
> -	return atomic_add_return(num, &fence_context_counter) - num;
> +	return atomic64_add_return(num, &fence_context_counter) - num;
>  }
>  EXPORT_SYMBOL(fence_context_alloc);
>  
> @@ -513,7 +513,7 @@ EXPORT_SYMBOL(fence_wait_any_timeout);
>   */
>  void
>  fence_init(struct fence *fence, const struct fence_ops *ops,
> -	     spinlock_t *lock, unsigned context, unsigned seqno)
> +	     spinlock_t *lock, u64 context, unsigned seqno)
>  {
>  	BUG_ON(!lock);
>  	BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 992f00b..da3d021 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2032,7 +2032,7 @@ struct amdgpu_device {
>  	struct amdgpu_irq_src		hpd_irq;
>  
>  	/* rings */
> -	unsigned			fence_context;
> +	u64				fence_context;
>  	unsigned			num_rings;
>  	struct amdgpu_ring		*rings[AMDGPU_MAX_RINGS];
>  	bool				ib_pool_ready;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index f5321e2..a69cdd5 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -125,7 +125,7 @@ struct etnaviv_gpu {
>  	u32 completed_fence;
>  	u32 retired_fence;
>  	wait_queue_head_t fence_event;
> -	unsigned int fence_context;
> +	u64 fence_context;
>  	spinlock_t fence_spinlock;
>  
>  	/* worker for handling active-list retiring: */
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h b/drivers/gpu/drm/nouveau/nouveau_fence.h
> index 2e3a62d..64c4ce7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.h
> @@ -57,7 +57,8 @@ struct nouveau_fence_priv {
>  	int  (*context_new)(struct nouveau_channel *);
>  	void (*context_del)(struct nouveau_channel *);
>  
> -	u32 contexts, context_base;
> +	u32 contexts;
> +	u64 context_base;
>  	bool uevent;
>  };
>  
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 80b24a4..5633ee3 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2386,7 +2386,7 @@ struct radeon_device {
>  	struct radeon_mman		mman;
>  	struct radeon_fence_driver	fence_drv[RADEON_NUM_RINGS];
>  	wait_queue_head_t		fence_queue;
> -	unsigned			fence_context;
> +	u64				fence_context;
>  	struct mutex			ring_lock;
>  	struct radeon_ring		ring[RADEON_NUM_RINGS];
>  	bool				ib_pool_ready;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index e959df6..26ac8e8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -46,7 +46,7 @@ struct vmw_fence_manager {
>  	bool goal_irq_on; /* Protected by @goal_irq_mutex */
>  	bool seqno_valid; /* Protected by @lock, and may not be set to true
>  			     without the @goal_irq_mutex held. */
> -	unsigned ctx;
> +	u64 ctx;
>  };
>  
>  struct vmw_user_fence {
> diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
> index d2a1734..ef1f7d4 100644
> --- a/drivers/staging/android/sync.h
> +++ b/drivers/staging/android/sync.h
> @@ -68,7 +68,8 @@ struct sync_timeline {
>  
>  	/* protected by child_list_lock */
>  	bool			destroyed;
> -	int			context, value;
> +	u64			context;
> +	int			value;
>  
>  	struct list_head	child_list_head;
>  	spinlock_t		child_list_lock;
> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index 2b17698..350caaa 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -75,7 +75,8 @@ struct fence {
>  	struct rcu_head rcu;
>  	struct list_head cb_list;
>  	spinlock_t *lock;
> -	unsigned context, seqno;
> +	u64 context;
> +	unsigned seqno;
>  	unsigned long flags;
>  	ktime_t timestamp;
>  	int status;
> @@ -178,7 +179,7 @@ struct fence_ops {
>  };
>  
>  void fence_init(struct fence *fence, const struct fence_ops *ops,
> -		spinlock_t *lock, unsigned context, unsigned seqno);
> +		spinlock_t *lock, u64 context, unsigned seqno);
>  
>  void fence_release(struct kref *kref);
>  void fence_free(struct fence *fence);
> @@ -352,7 +353,7 @@ static inline signed long fence_wait(struct fence *fence, bool intr)
>  	return ret < 0 ? ret : 0;
>  }
>  
> -unsigned fence_context_alloc(unsigned num);
> +u64 fence_context_alloc(unsigned num);
>  
>  #define FENCE_TRACE(f, fmt, args...) \
>  	do {								\
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list