Re: [PATCH] dma-buf/sync-file: Defer creation of sync_file->name

[Gustavo Padovan <gustavo@padovan.org>]

Hi Chris,

Thanks for the patch!

2017-05-12 Chris Wilson <chris@chris-wilson.co.uk>:

> Constructing the name takes the majority of the time for allocating a
> sync_file to wrap a fence, and the name is very rarely used (only via
> the sync_file status user interface). To reduce the impact on the common
> path (that of creating sync_file to pass around), defer the construction
> of the name until it is first used.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> ---
>  drivers/dma-buf/sync_file.c | 18 +++++++++++-------
>  include/linux/sync_file.h   |  2 +-
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 2321035f6204..2cccfa834778 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -82,11 +82,6 @@ struct sync_file *sync_file_create(struct dma_fence *fence)
>  
>  	sync_file->fence = dma_fence_get(fence);
>  
> -	snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d",
> -		 fence->ops->get_driver_name(fence),
> -		 fence->ops->get_timeline_name(fence), fence->context,
> -		 fence->seqno);
> -
>  	return sync_file;
>  }
>  EXPORT_SYMBOL(sync_file_create);
> @@ -268,7 +263,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>  		goto err;
>  	}
>  
> -	strlcpy(sync_file->name, name, sizeof(sync_file->name));
> +	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
>  	return sync_file;
>  
>  err:
> @@ -422,7 +417,16 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>  	}
>  
>  no_fences:
> -	strlcpy(info.name, sync_file->name, sizeof(info.name));
> +	if (!sync_file->user_name[0]) {
> +		scnprintf(sync_file->user_name,
> +			  sizeof(sync_file->user_name),
> +			  "%s-%s%llu-%d",
> +			  sync_file->fence->ops->get_driver_name(sync_file->fence),
> +			  sync_file->fence->ops->get_timeline_name(sync_file->fence),
> +			  sync_file->fence->context,
> +			  sync_file->fence->seqno);
> +	}
> +	strlcpy(info.name, sync_file->user_name, sizeof(info.name));
>  	info.status = dma_fence_is_signaled(sync_file->fence);
>  	info.num_fences = num_fences;
>  
> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
> index 3e3ab84fc4cd..0cbeff29f510 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -34,7 +34,7 @@
>  struct sync_file {
>  	struct file		*file;
>  	struct kref		kref;
> -	char			name[32];
> +	char			user_name[32];

Looks good to me, but please re-spin it fixing the issue reported by the
test bot then I can apply it.

Gustavo

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx