On Fri, May 12, 2017 at 07:55:42PM +0100, Chris Wilson wrote:
> 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.
>
> v2: Update kerneldoc (kbuild test robot)
> v3: sync_debug.c was peeking at the name
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx>
> ---
> drivers/dma-buf/sync_debug.c | 3 ++-
> drivers/dma-buf/sync_file.c | 34 +++++++++++++++++++++++++++-------
> include/linux/sync_file.h | 5 +++--
> 3 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> index 4b1731ee7458..9a93f1085c63 100644
> --- a/drivers/dma-buf/sync_debug.c
> +++ b/drivers/dma-buf/sync_debug.c
> @@ -134,7 +134,8 @@ static void sync_print_sync_file(struct seq_file *s,
> {
> int i;
>
> - seq_printf(s, "[%p] %s: %s\n", sync_file, sync_file->name,
> + seq_printf(s, "[%p] %s: %s\n", sync_file,
> + sync_file_get_name(sync_file),
> sync_status_str(dma_fence_get_status(sync_file->fence)));
>
> if (dma_fence_is_array(sync_file->fence)) {
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index c9eb4997cfcc..d105079ec45c 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -80,11 +80,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);
> @@ -129,6 +124,31 @@ struct dma_fence *sync_file_get_fence(int fd)
> }
> EXPORT_SYMBOL(sync_file_get_fence);
>
> +/**
> + * sync_file_get_name - get the name of the sync_file
> + * @sync_file: sync_file to get the fence from
> + *
> + * Each sync_file may have a name assigned either by the user (when merging
> + * sync_files together) or created from the fence it contains. However,
> + * construction of the name is deferred until first use.
> + *
> + * Returns: a string representing the name
> + */
> +char *sync_file_get_name(struct sync_file *sync_file)
> +{
> + 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);
> + }
This is mildly race. Do we care? Deserves at least a comment that we don't
care, with that ack: me.
-Daniel
> +
> + return sync_file->user_name;
> +}
> +
> static int sync_file_set_fence(struct sync_file *sync_file,
> struct dma_fence **fences, int num_fences)
> {
> @@ -266,7 +286,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:
> @@ -419,7 +439,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> }
>
> no_fences:
> - strlcpy(info.name, sync_file->name, sizeof(info.name));
> + strlcpy(info.name, sync_file_get_name(sync_file), 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 d37beefdfbd5..0f3f05325c84 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -23,7 +23,7 @@
> /**
> * struct sync_file - sync file to export to the userspace
> * @file: file representing this fence
> - * @name: name of sync_file. Useful for debugging
> + * @user_name: name of sync_file. Useful for debugging
> * @sync_file_list: membership in global file list
> * @wq: wait queue for fence signaling
> * @fence: fence with the fences in the sync_file
> @@ -31,7 +31,7 @@
> */
> struct sync_file {
> struct file *file;
> - char name[32];
> + char user_name[32];
> #ifdef CONFIG_DEBUG_FS
> struct list_head sync_file_list;
> #endif
> @@ -46,5 +46,6 @@ struct sync_file {
>
> struct sync_file *sync_file_create(struct dma_fence *fence);
> struct dma_fence *sync_file_get_fence(int fd);
> +char *sync_file_get_name(struct sync_file *sync_file);
>
> #endif /* _LINUX_SYNC_H */
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
