diff mbox series

[2/2] dma-buf/sync-file: fix warning about fence containers

Message ID	20220311110244.1245-2-christian.koenig@amd.com (mailing list archive)
State	Not Applicable
Headers	From: " =?utf-8?q?Christian_K=C3=B6nig?= " <ckoenig.leichtzumerken@gmail.com> To: sumit.semwal@linaro.org, gustavo@padovan.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org Cc: =?utf-8?q?Christian_K=C3=B6nig?= <christian.koenig@amd.com> Subject: [PATCH 2/2] dma-buf/sync-file: fix warning about fence containers Date: Fri, 11 Mar 2022 12:02:44 +0100 Message-Id: <20220311110244.1245-2-christian.koenig@amd.com> In-Reply-To: <20220311110244.1245-1-christian.koenig@amd.com> References: <20220311110244.1245-1-christian.koenig@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	[1/2] dma-buf: add dma_fence_unwrap \| [1/2] dma-buf: add dma_fence_unwrap [2/2] dma-buf/sync-file: fix warning about fence containers

Commit Message

Christian König March 11, 2022, 11:02 a.m. UTC

  The dma_fence_chain containers can show up in sync_files as well resulting in
warnings that those can't be added to dma_fence_array containers when merging
multiple sync_files together.

Solve this by using the dma_fence_unwrap iterator to deep dive into the
contained fences and then add those flatten out into a dma_fence_array.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/sync_file.c | 141 +++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 68 deletions(-)

Comments

kernel test robot March 11, 2022, 4:16 p.m. UTC | #1

Hi "Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 79b00034e9dcd2b065c1665c8b42f62b6b80a9be
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220312/202203120047.SyXpIs6H-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
        git checkout ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/dma-buf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/dma-buf/sync_file.c:8:
   include/linux/dma-fence-unwrap.h: In function 'dma_fence_unwrap_array':
   include/linux/dma-fence-unwrap.h:44:18: error: implicit declaration of function 'dma_fence_chain_contained'; did you mean 'dma_fence_chain_init'? [-Werror=implicit-function-declaration]
      44 |  cursor->array = dma_fence_chain_contained(cursor->chain);
         |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                  dma_fence_chain_init
>> include/linux/dma-fence-unwrap.h:44:16: warning: assignment to 'struct dma_fence *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      44 |  cursor->array = dma_fence_chain_contained(cursor->chain);
         |                ^
   include/linux/dma-fence-unwrap.h:46:9: error: implicit declaration of function 'dma_fence_array_first'; did you mean 'dma_fence_array_create'? [-Werror=implicit-function-declaration]
      46 |  return dma_fence_array_first(cursor->array);
         |         ^~~~~~~~~~~~~~~~~~~~~
         |         dma_fence_array_create
>> include/linux/dma-fence-unwrap.h:46:9: warning: returning 'int' from a function with return type 'struct dma_fence *' makes pointer from integer without a cast [-Wint-conversion]
      46 |  return dma_fence_array_first(cursor->array);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-fence-unwrap.h: In function 'dma_fence_unwrap_next':
   include/linux/dma-fence-unwrap.h:77:8: error: implicit declaration of function 'dma_fence_array_next'; did you mean 'dma_fence_unwrap_next'? [-Werror=implicit-function-declaration]
      77 |  tmp = dma_fence_array_next(cursor->array, cursor->index);
         |        ^~~~~~~~~~~~~~~~~~~~
         |        dma_fence_unwrap_next
   include/linux/dma-fence-unwrap.h:77:6: warning: assignment to 'struct dma_fence *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      77 |  tmp = dma_fence_array_next(cursor->array, cursor->index);
         |      ^
   cc1: some warnings being treated as errors


vim +44 include/linux/dma-fence-unwrap.h

088aa14c0f5cad Christian König 2022-03-11  33  
088aa14c0f5cad Christian König 2022-03-11  34  /**
088aa14c0f5cad Christian König 2022-03-11  35   * dma_fence_unwrap_array - helper to unwrap dma_fence_arrays
088aa14c0f5cad Christian König 2022-03-11  36   * @cursor: cursor to initialize
088aa14c0f5cad Christian König 2022-03-11  37   *
088aa14c0f5cad Christian König 2022-03-11  38   * Helper function to unwrap dma_fence_array containers, don't touch directly.
088aa14c0f5cad Christian König 2022-03-11  39   * Use dma_fence_unwrap_first/next instead.
088aa14c0f5cad Christian König 2022-03-11  40   */
088aa14c0f5cad Christian König 2022-03-11  41  static inline struct dma_fence *
088aa14c0f5cad Christian König 2022-03-11  42  dma_fence_unwrap_array(struct dma_fence_unwrap * cursor)
088aa14c0f5cad Christian König 2022-03-11  43  {
088aa14c0f5cad Christian König 2022-03-11 @44  	cursor->array = dma_fence_chain_contained(cursor->chain);
088aa14c0f5cad Christian König 2022-03-11  45  	cursor->index = 0;
088aa14c0f5cad Christian König 2022-03-11 @46  	return dma_fence_array_first(cursor->array);
088aa14c0f5cad Christian König 2022-03-11  47  }
088aa14c0f5cad Christian König 2022-03-11  48  

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

kernel test robot March 11, 2022, 5:59 p.m. UTC | #2

Hi "Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 79b00034e9dcd2b065c1665c8b42f62b6b80a9be
config: arm64-randconfig-r014-20220310 (https://download.01.org/0day-ci/archive/20220312/202203120115.Qe4GABIV-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
        git checkout ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/dma-buf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/dma-buf/sync_file.c:8:
   include/linux/dma-fence-unwrap.h:44:18: error: implicit declaration of function 'dma_fence_chain_contained' [-Werror,-Wimplicit-function-declaration]
           cursor->array = dma_fence_chain_contained(cursor->chain);
                           ^
   include/linux/dma-fence-unwrap.h:44:18: note: did you mean 'dma_fence_chain_init'?
   include/linux/dma-fence-chain.h:108:6: note: 'dma_fence_chain_init' declared here
   void dma_fence_chain_init(struct dma_fence_chain *chain,
        ^
   In file included from drivers/dma-buf/sync_file.c:8:
>> include/linux/dma-fence-unwrap.h:44:16: warning: incompatible integer to pointer conversion assigning to 'struct dma_fence *' from 'int' [-Wint-conversion]
           cursor->array = dma_fence_chain_contained(cursor->chain);
                         ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-fence-unwrap.h:46:9: error: implicit declaration of function 'dma_fence_array_first' [-Werror,-Wimplicit-function-declaration]
           return dma_fence_array_first(cursor->array);
                  ^
   include/linux/dma-fence-unwrap.h:46:9: note: did you mean 'dma_fence_array_create'?
   include/linux/dma-fence-array.h:77:25: note: 'dma_fence_array_create' declared here
   struct dma_fence_array *dma_fence_array_create(int num_fences,
                           ^
   In file included from drivers/dma-buf/sync_file.c:8:
>> include/linux/dma-fence-unwrap.h:46:9: warning: incompatible integer to pointer conversion returning 'int' from a function with result type 'struct dma_fence *' [-Wint-conversion]
           return dma_fence_array_first(cursor->array);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-fence-unwrap.h:77:8: error: implicit declaration of function 'dma_fence_array_next' [-Werror,-Wimplicit-function-declaration]
           tmp = dma_fence_array_next(cursor->array, cursor->index);
                 ^
   include/linux/dma-fence-unwrap.h:77:6: warning: incompatible integer to pointer conversion assigning to 'struct dma_fence *' from 'int' [-Wint-conversion]
           tmp = dma_fence_array_next(cursor->array, cursor->index);
               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings and 3 errors generated.


vim +44 include/linux/dma-fence-unwrap.h

088aa14c0f5cad Christian König 2022-03-11  33  
088aa14c0f5cad Christian König 2022-03-11  34  /**
088aa14c0f5cad Christian König 2022-03-11  35   * dma_fence_unwrap_array - helper to unwrap dma_fence_arrays
088aa14c0f5cad Christian König 2022-03-11  36   * @cursor: cursor to initialize
088aa14c0f5cad Christian König 2022-03-11  37   *
088aa14c0f5cad Christian König 2022-03-11  38   * Helper function to unwrap dma_fence_array containers, don't touch directly.
088aa14c0f5cad Christian König 2022-03-11  39   * Use dma_fence_unwrap_first/next instead.
088aa14c0f5cad Christian König 2022-03-11  40   */
088aa14c0f5cad Christian König 2022-03-11  41  static inline struct dma_fence *
088aa14c0f5cad Christian König 2022-03-11  42  dma_fence_unwrap_array(struct dma_fence_unwrap * cursor)
088aa14c0f5cad Christian König 2022-03-11  43  {
088aa14c0f5cad Christian König 2022-03-11 @44  	cursor->array = dma_fence_chain_contained(cursor->chain);
088aa14c0f5cad Christian König 2022-03-11  45  	cursor->index = 0;
088aa14c0f5cad Christian König 2022-03-11 @46  	return dma_fence_array_first(cursor->array);
088aa14c0f5cad Christian König 2022-03-11  47  }
088aa14c0f5cad Christian König 2022-03-11  48  

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

kernel test robot March 11, 2022, 6:51 p.m. UTC | #3

Hi "Christian,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 79b00034e9dcd2b065c1665c8b42f62b6b80a9be
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220312/202203120217.BFa438j9-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-K-nig/dma-buf-add-dma_fence_unwrap/20220311-190352
        git checkout ca3584ac05c4a450e69b1c6bcb0672b5ab026c7c
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/dma-buf/sync_file.c:8:
   include/linux/dma-fence-unwrap.h: In function 'dma_fence_unwrap_array':
>> include/linux/dma-fence-unwrap.h:44:18: error: implicit declaration of function 'dma_fence_chain_contained'; did you mean 'dma_fence_chain_init'? [-Werror=implicit-function-declaration]
      44 |  cursor->array = dma_fence_chain_contained(cursor->chain);
         |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                  dma_fence_chain_init
   include/linux/dma-fence-unwrap.h:44:16: warning: assignment to 'struct dma_fence *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      44 |  cursor->array = dma_fence_chain_contained(cursor->chain);
         |                ^
>> include/linux/dma-fence-unwrap.h:46:9: error: implicit declaration of function 'dma_fence_array_first'; did you mean 'dma_fence_array_create'? [-Werror=implicit-function-declaration]
      46 |  return dma_fence_array_first(cursor->array);
         |         ^~~~~~~~~~~~~~~~~~~~~
         |         dma_fence_array_create
   include/linux/dma-fence-unwrap.h:46:9: warning: returning 'int' from a function with return type 'struct dma_fence *' makes pointer from integer without a cast [-Wint-conversion]
      46 |  return dma_fence_array_first(cursor->array);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-fence-unwrap.h: In function 'dma_fence_unwrap_next':
>> include/linux/dma-fence-unwrap.h:77:8: error: implicit declaration of function 'dma_fence_array_next'; did you mean 'dma_fence_unwrap_next'? [-Werror=implicit-function-declaration]
      77 |  tmp = dma_fence_array_next(cursor->array, cursor->index);
         |        ^~~~~~~~~~~~~~~~~~~~
         |        dma_fence_unwrap_next
   include/linux/dma-fence-unwrap.h:77:6: warning: assignment to 'struct dma_fence *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      77 |  tmp = dma_fence_array_next(cursor->array, cursor->index);
         |      ^
   cc1: some warnings being treated as errors


vim +44 include/linux/dma-fence-unwrap.h

088aa14c0f5cad Christian König 2022-03-11  33  
088aa14c0f5cad Christian König 2022-03-11  34  /**
088aa14c0f5cad Christian König 2022-03-11  35   * dma_fence_unwrap_array - helper to unwrap dma_fence_arrays
088aa14c0f5cad Christian König 2022-03-11  36   * @cursor: cursor to initialize
088aa14c0f5cad Christian König 2022-03-11  37   *
088aa14c0f5cad Christian König 2022-03-11  38   * Helper function to unwrap dma_fence_array containers, don't touch directly.
088aa14c0f5cad Christian König 2022-03-11  39   * Use dma_fence_unwrap_first/next instead.
088aa14c0f5cad Christian König 2022-03-11  40   */
088aa14c0f5cad Christian König 2022-03-11  41  static inline struct dma_fence *
088aa14c0f5cad Christian König 2022-03-11  42  dma_fence_unwrap_array(struct dma_fence_unwrap * cursor)
088aa14c0f5cad Christian König 2022-03-11  43  {
088aa14c0f5cad Christian König 2022-03-11 @44  	cursor->array = dma_fence_chain_contained(cursor->chain);
088aa14c0f5cad Christian König 2022-03-11  45  	cursor->index = 0;
088aa14c0f5cad Christian König 2022-03-11 @46  	return dma_fence_array_first(cursor->array);
088aa14c0f5cad Christian König 2022-03-11  47  }
088aa14c0f5cad Christian König 2022-03-11  48  
088aa14c0f5cad Christian König 2022-03-11  49  /**
088aa14c0f5cad Christian König 2022-03-11  50   * dma_fence_unwrap_first - return the first fence from fence containers
088aa14c0f5cad Christian König 2022-03-11  51   * @head: the entrypoint into the containers
088aa14c0f5cad Christian König 2022-03-11  52   * @cursor: current position inside the containers
088aa14c0f5cad Christian König 2022-03-11  53   *
088aa14c0f5cad Christian König 2022-03-11  54   * Unwraps potential dma_fence_chain/dma_fence_array containers and return the
088aa14c0f5cad Christian König 2022-03-11  55   * first fence.
088aa14c0f5cad Christian König 2022-03-11  56   */
088aa14c0f5cad Christian König 2022-03-11  57  static inline struct dma_fence *
088aa14c0f5cad Christian König 2022-03-11  58  dma_fence_unwrap_first(struct dma_fence *head, struct dma_fence_unwrap *cursor)
088aa14c0f5cad Christian König 2022-03-11  59  {
088aa14c0f5cad Christian König 2022-03-11  60  	cursor->chain = dma_fence_get(head);
088aa14c0f5cad Christian König 2022-03-11  61  	return dma_fence_unwrap_array(cursor);
088aa14c0f5cad Christian König 2022-03-11  62  }
088aa14c0f5cad Christian König 2022-03-11  63  
088aa14c0f5cad Christian König 2022-03-11  64  /**
088aa14c0f5cad Christian König 2022-03-11  65   * dma_fence_unwrap_next - return the next fence from a fence containers
088aa14c0f5cad Christian König 2022-03-11  66   * @cursor: current position inside the containers
088aa14c0f5cad Christian König 2022-03-11  67   *
088aa14c0f5cad Christian König 2022-03-11  68   * Continue unwrapping the dma_fence_chain/dma_fence_array containers and return
088aa14c0f5cad Christian König 2022-03-11  69   * the next fence from them.
088aa14c0f5cad Christian König 2022-03-11  70   */
088aa14c0f5cad Christian König 2022-03-11  71  static inline struct dma_fence *
088aa14c0f5cad Christian König 2022-03-11  72  dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
088aa14c0f5cad Christian König 2022-03-11  73  {
088aa14c0f5cad Christian König 2022-03-11  74  	struct dma_fence *tmp;
088aa14c0f5cad Christian König 2022-03-11  75  
088aa14c0f5cad Christian König 2022-03-11  76  	++cursor->index;
088aa14c0f5cad Christian König 2022-03-11 @77  	tmp = dma_fence_array_next(cursor->array, cursor->index);
088aa14c0f5cad Christian König 2022-03-11  78  	if (tmp)
088aa14c0f5cad Christian König 2022-03-11  79  		return tmp;
088aa14c0f5cad Christian König 2022-03-11  80  
088aa14c0f5cad Christian König 2022-03-11  81  	cursor->chain = dma_fence_chain_walk(cursor->chain);
088aa14c0f5cad Christian König 2022-03-11  82  	return dma_fence_unwrap_array(cursor);
088aa14c0f5cad Christian König 2022-03-11  83  }
088aa14c0f5cad Christian König 2022-03-11  84  

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Daniel Vetter March 25, 2022, 10:13 a.m. UTC | #4

On Fri, Mar 11, 2022 at 12:02:44PM +0100, Christian König wrote:
> The dma_fence_chain containers can show up in sync_files as well resulting in
> warnings that those can't be added to dma_fence_array containers when merging
> multiple sync_files together.
> 
> Solve this by using the dma_fence_unwrap iterator to deep dive into the
> contained fences and then add those flatten out into a dma_fence_array.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

I have no idea why we try to keep fences sorted, but oh well it looks like
the merging is done correctly.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/dma-buf/sync_file.c | 141 +++++++++++++++++++-----------------
>  1 file changed, 73 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 394e6e1e9686..b8dea4ec123b 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2012 Google, Inc.
>   */
>  
> +#include <linux/dma-fence-unwrap.h>
>  #include <linux/export.h>
>  #include <linux/file.h>
>  #include <linux/fs.h>
> @@ -172,20 +173,6 @@ static int sync_file_set_fence(struct sync_file *sync_file,
>  	return 0;
>  }
>  
> -static struct dma_fence **get_fences(struct sync_file *sync_file,
> -				     int *num_fences)
> -{
> -	if (dma_fence_is_array(sync_file->fence)) {
> -		struct dma_fence_array *array = to_dma_fence_array(sync_file->fence);
> -
> -		*num_fences = array->num_fences;
> -		return array->fences;
> -	}
> -
> -	*num_fences = 1;
> -	return &sync_file->fence;
> -}
> -
>  static void add_fence(struct dma_fence **fences,
>  		      int *i, struct dma_fence *fence)
>  {
> @@ -210,86 +197,97 @@ static void add_fence(struct dma_fence **fences,
>  static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>  					 struct sync_file *b)
>  {
> +	struct dma_fence *a_fence, *b_fence, **fences;
> +	struct dma_fence_unwrap a_iter, b_iter;
> +	unsigned int index, num_fences;
>  	struct sync_file *sync_file;
> -	struct dma_fence **fences = NULL, **nfences, **a_fences, **b_fences;
> -	int i = 0, i_a, i_b, num_fences, a_num_fences, b_num_fences;
>  
>  	sync_file = sync_file_alloc();
>  	if (!sync_file)
>  		return NULL;
>  
> -	a_fences = get_fences(a, &a_num_fences);
> -	b_fences = get_fences(b, &b_num_fences);
> -	if (a_num_fences > INT_MAX - b_num_fences)
> -		goto err;
> +	num_fences = 0;
> +	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
> +		++num_fences;
> +	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
> +		++num_fences;
>  
> -	num_fences = a_num_fences + b_num_fences;
> +	if (num_fences > INT_MAX)
> +		goto err_free_sync_file;
>  
>  	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
>  	if (!fences)
> -		goto err;
> +		goto err_free_sync_file;
>  
>  	/*
> -	 * Assume sync_file a and b are both ordered and have no
> -	 * duplicates with the same context.
> +	 * We can't guarantee that fences in both a and b are ordered, but it is
> +	 * still quite likely.
>  	 *
> -	 * If a sync_file can only be created with sync_file_merge
> -	 * and sync_file_create, this is a reasonable assumption.
> +	 * So attempt to order the fences as we pass over them and merge fences
> +	 * with the same context.
>  	 */
> -	for (i_a = i_b = 0; i_a < a_num_fences && i_b < b_num_fences; ) {
> -		struct dma_fence *pt_a = a_fences[i_a];
> -		struct dma_fence *pt_b = b_fences[i_b];
>  
> -		if (pt_a->context < pt_b->context) {
> -			add_fence(fences, &i, pt_a);
> +	index = 0;
> +	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
> +	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
> +	     a_fence || b_fence; ) {
> +
> +		if (!b_fence) {
> +			add_fence(fences, &index, a_fence);
> +			a_fence = dma_fence_unwrap_next(&a_iter);
> +
> +		} else if (!a_fence) {
> +			add_fence(fences, &index, b_fence);
> +			b_fence = dma_fence_unwrap_next(&b_iter);
> +
> +		} else if (a_fence->context < b_fence->context) {
> +			add_fence(fences, &index, a_fence);
> +			a_fence = dma_fence_unwrap_next(&a_iter);
>  
> -			i_a++;
> -		} else if (pt_a->context > pt_b->context) {
> -			add_fence(fences, &i, pt_b);
> +		} else if (b_fence->context < a_fence->context) {
> +			add_fence(fences, &index, b_fence);
> +			b_fence = dma_fence_unwrap_next(&b_iter);
> +
> +		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
> +						a_fence->ops)) {
> +			add_fence(fences, &index, a_fence);
> +			a_fence = dma_fence_unwrap_next(&a_iter);
> +			b_fence = dma_fence_unwrap_next(&b_iter);
>  
> -			i_b++;
>  		} else {
> -			if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno,
> -						 pt_a->ops))
> -				add_fence(fences, &i, pt_a);
> -			else
> -				add_fence(fences, &i, pt_b);
> -
> -			i_a++;
> -			i_b++;
> +			add_fence(fences, &index, b_fence);
> +			a_fence = dma_fence_unwrap_next(&a_iter);
> +			b_fence = dma_fence_unwrap_next(&b_iter);
>  		}
>  	}
>  
> -	for (; i_a < a_num_fences; i_a++)
> -		add_fence(fences, &i, a_fences[i_a]);
> -
> -	for (; i_b < b_num_fences; i_b++)
> -		add_fence(fences, &i, b_fences[i_b]);
> -
> -	if (i == 0)
> -		fences[i++] = dma_fence_get(a_fences[0]);
> +	if (index == 0)
> +		add_fence(fences, &index, dma_fence_get_stub());
>  
> -	if (num_fences > i) {
> -		nfences = krealloc_array(fences, i, sizeof(*fences), GFP_KERNEL);
> -		if (!nfences)
> -			goto err;
> +	if (num_fences > index) {
> +		struct dma_fence **tmp;
>  
> -		fences = nfences;
> +		/* Keep going even when reducing the size failed */
> +		tmp = krealloc_array(fences, index, sizeof(*fences),
> +				     GFP_KERNEL);
> +		if (tmp)
> +			fences = tmp;
>  	}
>  
> -	if (sync_file_set_fence(sync_file, fences, i) < 0)
> -		goto err;
> +	if (sync_file_set_fence(sync_file, fences, index) < 0)
> +		goto err_put_fences;
>  
>  	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
>  	return sync_file;
>  
> -err:
> -	while (i)
> -		dma_fence_put(fences[--i]);
> +err_put_fences:
> +	while (index)
> +		dma_fence_put(fences[--index]);
>  	kfree(fences);
> +
> +err_free_sync_file:
>  	fput(sync_file->file);
>  	return NULL;
> -
>  }
>  
>  static int sync_file_release(struct inode *inode, struct file *file)
> @@ -398,11 +396,13 @@ static int sync_fill_fence_info(struct dma_fence *fence,
>  static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>  				       unsigned long arg)
>  {
> -	struct sync_file_info info;
>  	struct sync_fence_info *fence_info = NULL;
> -	struct dma_fence **fences;
> +	struct dma_fence_unwrap iter;
> +	struct sync_file_info info;
> +	unsigned int num_fences;
> +	struct dma_fence *fence;
> +	int ret;
>  	__u32 size;
> -	int num_fences, ret, i;
>  
>  	if (copy_from_user(&info, (void __user *)arg, sizeof(info)))
>  		return -EFAULT;
> @@ -410,7 +410,9 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>  	if (info.flags || info.pad)
>  		return -EINVAL;
>  
> -	fences = get_fences(sync_file, &num_fences);
> +	num_fences = 0;
> +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence)
> +		++num_fences;
>  
>  	/*
>  	 * Passing num_fences = 0 means that userspace doesn't want to
> @@ -433,8 +435,11 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>  	if (!fence_info)
>  		return -ENOMEM;
>  
> -	for (i = 0; i < num_fences; i++) {
> -		int status = sync_fill_fence_info(fences[i], &fence_info[i]);
> +	num_fences = 0;
> +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence) {
> +		int status;
> +
> +		status = sync_fill_fence_info(fence, &fence_info[num_fences++]);
>  		info.status = info.status <= 0 ? info.status : status;
>  	}
>  
> -- 
> 2.25.1
>

Christian König March 25, 2022, 10:35 a.m. UTC | #5

Am 25.03.22 um 11:13 schrieb Daniel Vetter:
> On Fri, Mar 11, 2022 at 12:02:44PM +0100, Christian König wrote:
>> The dma_fence_chain containers can show up in sync_files as well resulting in
>> warnings that those can't be added to dma_fence_array containers when merging
>> multiple sync_files together.
>>
>> Solve this by using the dma_fence_unwrap iterator to deep dive into the
>> contained fences and then add those flatten out into a dma_fence_array.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> I have no idea why we try to keep fences sorted, but oh well it looks like
> the merging is done correctly.

To be honest I don't fully know either.

Keeping the array sorted by context allows to merge it without adding 
duplicates, but adding duplicates is not an extra overhead to begin with 
because we always allocate memory for the worst case anyway.

Just keeping it around for now.

> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks,
Christian.

>
>> ---
>>   drivers/dma-buf/sync_file.c | 141 +++++++++++++++++++-----------------
>>   1 file changed, 73 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
>> index 394e6e1e9686..b8dea4ec123b 100644
>> --- a/drivers/dma-buf/sync_file.c
>> +++ b/drivers/dma-buf/sync_file.c
>> @@ -5,6 +5,7 @@
>>    * Copyright (C) 2012 Google, Inc.
>>    */
>>   
>> +#include <linux/dma-fence-unwrap.h>
>>   #include <linux/export.h>
>>   #include <linux/file.h>
>>   #include <linux/fs.h>
>> @@ -172,20 +173,6 @@ static int sync_file_set_fence(struct sync_file *sync_file,
>>   	return 0;
>>   }
>>   
>> -static struct dma_fence **get_fences(struct sync_file *sync_file,
>> -				     int *num_fences)
>> -{
>> -	if (dma_fence_is_array(sync_file->fence)) {
>> -		struct dma_fence_array *array = to_dma_fence_array(sync_file->fence);
>> -
>> -		*num_fences = array->num_fences;
>> -		return array->fences;
>> -	}
>> -
>> -	*num_fences = 1;
>> -	return &sync_file->fence;
>> -}
>> -
>>   static void add_fence(struct dma_fence **fences,
>>   		      int *i, struct dma_fence *fence)
>>   {
>> @@ -210,86 +197,97 @@ static void add_fence(struct dma_fence **fences,
>>   static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>>   					 struct sync_file *b)
>>   {
>> +	struct dma_fence *a_fence, *b_fence, **fences;
>> +	struct dma_fence_unwrap a_iter, b_iter;
>> +	unsigned int index, num_fences;
>>   	struct sync_file *sync_file;
>> -	struct dma_fence **fences = NULL, **nfences, **a_fences, **b_fences;
>> -	int i = 0, i_a, i_b, num_fences, a_num_fences, b_num_fences;
>>   
>>   	sync_file = sync_file_alloc();
>>   	if (!sync_file)
>>   		return NULL;
>>   
>> -	a_fences = get_fences(a, &a_num_fences);
>> -	b_fences = get_fences(b, &b_num_fences);
>> -	if (a_num_fences > INT_MAX - b_num_fences)
>> -		goto err;
>> +	num_fences = 0;
>> +	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
>> +		++num_fences;
>> +	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
>> +		++num_fences;
>>   
>> -	num_fences = a_num_fences + b_num_fences;
>> +	if (num_fences > INT_MAX)
>> +		goto err_free_sync_file;
>>   
>>   	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
>>   	if (!fences)
>> -		goto err;
>> +		goto err_free_sync_file;
>>   
>>   	/*
>> -	 * Assume sync_file a and b are both ordered and have no
>> -	 * duplicates with the same context.
>> +	 * We can't guarantee that fences in both a and b are ordered, but it is
>> +	 * still quite likely.
>>   	 *
>> -	 * If a sync_file can only be created with sync_file_merge
>> -	 * and sync_file_create, this is a reasonable assumption.
>> +	 * So attempt to order the fences as we pass over them and merge fences
>> +	 * with the same context.
>>   	 */
>> -	for (i_a = i_b = 0; i_a < a_num_fences && i_b < b_num_fences; ) {
>> -		struct dma_fence *pt_a = a_fences[i_a];
>> -		struct dma_fence *pt_b = b_fences[i_b];
>>   
>> -		if (pt_a->context < pt_b->context) {
>> -			add_fence(fences, &i, pt_a);
>> +	index = 0;
>> +	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
>> +	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
>> +	     a_fence || b_fence; ) {
>> +
>> +		if (!b_fence) {
>> +			add_fence(fences, &index, a_fence);
>> +			a_fence = dma_fence_unwrap_next(&a_iter);
>> +
>> +		} else if (!a_fence) {
>> +			add_fence(fences, &index, b_fence);
>> +			b_fence = dma_fence_unwrap_next(&b_iter);
>> +
>> +		} else if (a_fence->context < b_fence->context) {
>> +			add_fence(fences, &index, a_fence);
>> +			a_fence = dma_fence_unwrap_next(&a_iter);
>>   
>> -			i_a++;
>> -		} else if (pt_a->context > pt_b->context) {
>> -			add_fence(fences, &i, pt_b);
>> +		} else if (b_fence->context < a_fence->context) {
>> +			add_fence(fences, &index, b_fence);
>> +			b_fence = dma_fence_unwrap_next(&b_iter);
>> +
>> +		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
>> +						a_fence->ops)) {
>> +			add_fence(fences, &index, a_fence);
>> +			a_fence = dma_fence_unwrap_next(&a_iter);
>> +			b_fence = dma_fence_unwrap_next(&b_iter);
>>   
>> -			i_b++;
>>   		} else {
>> -			if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno,
>> -						 pt_a->ops))
>> -				add_fence(fences, &i, pt_a);
>> -			else
>> -				add_fence(fences, &i, pt_b);
>> -
>> -			i_a++;
>> -			i_b++;
>> +			add_fence(fences, &index, b_fence);
>> +			a_fence = dma_fence_unwrap_next(&a_iter);
>> +			b_fence = dma_fence_unwrap_next(&b_iter);
>>   		}
>>   	}
>>   
>> -	for (; i_a < a_num_fences; i_a++)
>> -		add_fence(fences, &i, a_fences[i_a]);
>> -
>> -	for (; i_b < b_num_fences; i_b++)
>> -		add_fence(fences, &i, b_fences[i_b]);
>> -
>> -	if (i == 0)
>> -		fences[i++] = dma_fence_get(a_fences[0]);
>> +	if (index == 0)
>> +		add_fence(fences, &index, dma_fence_get_stub());
>>   
>> -	if (num_fences > i) {
>> -		nfences = krealloc_array(fences, i, sizeof(*fences), GFP_KERNEL);
>> -		if (!nfences)
>> -			goto err;
>> +	if (num_fences > index) {
>> +		struct dma_fence **tmp;
>>   
>> -		fences = nfences;
>> +		/* Keep going even when reducing the size failed */
>> +		tmp = krealloc_array(fences, index, sizeof(*fences),
>> +				     GFP_KERNEL);
>> +		if (tmp)
>> +			fences = tmp;
>>   	}
>>   
>> -	if (sync_file_set_fence(sync_file, fences, i) < 0)
>> -		goto err;
>> +	if (sync_file_set_fence(sync_file, fences, index) < 0)
>> +		goto err_put_fences;
>>   
>>   	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
>>   	return sync_file;
>>   
>> -err:
>> -	while (i)
>> -		dma_fence_put(fences[--i]);
>> +err_put_fences:
>> +	while (index)
>> +		dma_fence_put(fences[--index]);
>>   	kfree(fences);
>> +
>> +err_free_sync_file:
>>   	fput(sync_file->file);
>>   	return NULL;
>> -
>>   }
>>   
>>   static int sync_file_release(struct inode *inode, struct file *file)
>> @@ -398,11 +396,13 @@ static int sync_fill_fence_info(struct dma_fence *fence,
>>   static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>>   				       unsigned long arg)
>>   {
>> -	struct sync_file_info info;
>>   	struct sync_fence_info *fence_info = NULL;
>> -	struct dma_fence **fences;
>> +	struct dma_fence_unwrap iter;
>> +	struct sync_file_info info;
>> +	unsigned int num_fences;
>> +	struct dma_fence *fence;
>> +	int ret;
>>   	__u32 size;
>> -	int num_fences, ret, i;
>>   
>>   	if (copy_from_user(&info, (void __user *)arg, sizeof(info)))
>>   		return -EFAULT;
>> @@ -410,7 +410,9 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>>   	if (info.flags || info.pad)
>>   		return -EINVAL;
>>   
>> -	fences = get_fences(sync_file, &num_fences);
>> +	num_fences = 0;
>> +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence)
>> +		++num_fences;
>>   
>>   	/*
>>   	 * Passing num_fences = 0 means that userspace doesn't want to
>> @@ -433,8 +435,11 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>>   	if (!fence_info)
>>   		return -ENOMEM;
>>   
>> -	for (i = 0; i < num_fences; i++) {
>> -		int status = sync_fill_fence_info(fences[i], &fence_info[i]);
>> +	num_fences = 0;
>> +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence) {
>> +		int status;
>> +
>> +		status = sync_fill_fence_info(fence, &fence_info[num_fences++]);
>>   		info.status = info.status <= 0 ? info.status : status;
>>   	}
>>   
>> -- 
>> 2.25.1
>>

Daniel Vetter March 25, 2022, 7:28 p.m. UTC | #6

On Fri, Mar 25, 2022 at 11:35:49AM +0100, Christian König wrote:
> Am 25.03.22 um 11:13 schrieb Daniel Vetter:
> > On Fri, Mar 11, 2022 at 12:02:44PM +0100, Christian König wrote:
> > > The dma_fence_chain containers can show up in sync_files as well resulting in
> > > warnings that those can't be added to dma_fence_array containers when merging
> > > multiple sync_files together.
> > > 
> > > Solve this by using the dma_fence_unwrap iterator to deep dive into the
> > > contained fences and then add those flatten out into a dma_fence_array.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > I have no idea why we try to keep fences sorted, but oh well it looks like
> > the merging is done correctly.
> 
> To be honest I don't fully know either.
> 
> Keeping the array sorted by context allows to merge it without adding
> duplicates, but adding duplicates is not an extra overhead to begin with
> because we always allocate memory for the worst case anyway.
> 
> Just keeping it around for now.

Hm I guess if we want to keep that we could make that an invariant of dma
fence arrays, i.e. sorted and deduplicated. Usually there should be few
enough fences that de-duping shouldn't be expensive really.

But no idea whether that's worth it.
-Daniel

> 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Thanks,
> Christian.
> 
> > 
> > > ---
> > >   drivers/dma-buf/sync_file.c | 141 +++++++++++++++++++-----------------
> > >   1 file changed, 73 insertions(+), 68 deletions(-)
> > > 
> > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > > index 394e6e1e9686..b8dea4ec123b 100644
> > > --- a/drivers/dma-buf/sync_file.c
> > > +++ b/drivers/dma-buf/sync_file.c
> > > @@ -5,6 +5,7 @@
> > >    * Copyright (C) 2012 Google, Inc.
> > >    */
> > > +#include <linux/dma-fence-unwrap.h>
> > >   #include <linux/export.h>
> > >   #include <linux/file.h>
> > >   #include <linux/fs.h>
> > > @@ -172,20 +173,6 @@ static int sync_file_set_fence(struct sync_file *sync_file,
> > >   	return 0;
> > >   }
> > > -static struct dma_fence **get_fences(struct sync_file *sync_file,
> > > -				     int *num_fences)
> > > -{
> > > -	if (dma_fence_is_array(sync_file->fence)) {
> > > -		struct dma_fence_array *array = to_dma_fence_array(sync_file->fence);
> > > -
> > > -		*num_fences = array->num_fences;
> > > -		return array->fences;
> > > -	}
> > > -
> > > -	*num_fences = 1;
> > > -	return &sync_file->fence;
> > > -}
> > > -
> > >   static void add_fence(struct dma_fence **fences,
> > >   		      int *i, struct dma_fence *fence)
> > >   {
> > > @@ -210,86 +197,97 @@ static void add_fence(struct dma_fence **fences,
> > >   static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
> > >   					 struct sync_file *b)
> > >   {
> > > +	struct dma_fence *a_fence, *b_fence, **fences;
> > > +	struct dma_fence_unwrap a_iter, b_iter;
> > > +	unsigned int index, num_fences;
> > >   	struct sync_file *sync_file;
> > > -	struct dma_fence **fences = NULL, **nfences, **a_fences, **b_fences;
> > > -	int i = 0, i_a, i_b, num_fences, a_num_fences, b_num_fences;
> > >   	sync_file = sync_file_alloc();
> > >   	if (!sync_file)
> > >   		return NULL;
> > > -	a_fences = get_fences(a, &a_num_fences);
> > > -	b_fences = get_fences(b, &b_num_fences);
> > > -	if (a_num_fences > INT_MAX - b_num_fences)
> > > -		goto err;
> > > +	num_fences = 0;
> > > +	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
> > > +		++num_fences;
> > > +	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
> > > +		++num_fences;
> > > -	num_fences = a_num_fences + b_num_fences;
> > > +	if (num_fences > INT_MAX)
> > > +		goto err_free_sync_file;
> > >   	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
> > >   	if (!fences)
> > > -		goto err;
> > > +		goto err_free_sync_file;
> > >   	/*
> > > -	 * Assume sync_file a and b are both ordered and have no
> > > -	 * duplicates with the same context.
> > > +	 * We can't guarantee that fences in both a and b are ordered, but it is
> > > +	 * still quite likely.
> > >   	 *
> > > -	 * If a sync_file can only be created with sync_file_merge
> > > -	 * and sync_file_create, this is a reasonable assumption.
> > > +	 * So attempt to order the fences as we pass over them and merge fences
> > > +	 * with the same context.
> > >   	 */
> > > -	for (i_a = i_b = 0; i_a < a_num_fences && i_b < b_num_fences; ) {
> > > -		struct dma_fence *pt_a = a_fences[i_a];
> > > -		struct dma_fence *pt_b = b_fences[i_b];
> > > -		if (pt_a->context < pt_b->context) {
> > > -			add_fence(fences, &i, pt_a);
> > > +	index = 0;
> > > +	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
> > > +	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
> > > +	     a_fence || b_fence; ) {
> > > +
> > > +		if (!b_fence) {
> > > +			add_fence(fences, &index, a_fence);
> > > +			a_fence = dma_fence_unwrap_next(&a_iter);
> > > +
> > > +		} else if (!a_fence) {
> > > +			add_fence(fences, &index, b_fence);
> > > +			b_fence = dma_fence_unwrap_next(&b_iter);
> > > +
> > > +		} else if (a_fence->context < b_fence->context) {
> > > +			add_fence(fences, &index, a_fence);
> > > +			a_fence = dma_fence_unwrap_next(&a_iter);
> > > -			i_a++;
> > > -		} else if (pt_a->context > pt_b->context) {
> > > -			add_fence(fences, &i, pt_b);
> > > +		} else if (b_fence->context < a_fence->context) {
> > > +			add_fence(fences, &index, b_fence);
> > > +			b_fence = dma_fence_unwrap_next(&b_iter);
> > > +
> > > +		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
> > > +						a_fence->ops)) {
> > > +			add_fence(fences, &index, a_fence);
> > > +			a_fence = dma_fence_unwrap_next(&a_iter);
> > > +			b_fence = dma_fence_unwrap_next(&b_iter);
> > > -			i_b++;
> > >   		} else {
> > > -			if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno,
> > > -						 pt_a->ops))
> > > -				add_fence(fences, &i, pt_a);
> > > -			else
> > > -				add_fence(fences, &i, pt_b);
> > > -
> > > -			i_a++;
> > > -			i_b++;
> > > +			add_fence(fences, &index, b_fence);
> > > +			a_fence = dma_fence_unwrap_next(&a_iter);
> > > +			b_fence = dma_fence_unwrap_next(&b_iter);
> > >   		}
> > >   	}
> > > -	for (; i_a < a_num_fences; i_a++)
> > > -		add_fence(fences, &i, a_fences[i_a]);
> > > -
> > > -	for (; i_b < b_num_fences; i_b++)
> > > -		add_fence(fences, &i, b_fences[i_b]);
> > > -
> > > -	if (i == 0)
> > > -		fences[i++] = dma_fence_get(a_fences[0]);
> > > +	if (index == 0)
> > > +		add_fence(fences, &index, dma_fence_get_stub());
> > > -	if (num_fences > i) {
> > > -		nfences = krealloc_array(fences, i, sizeof(*fences), GFP_KERNEL);
> > > -		if (!nfences)
> > > -			goto err;
> > > +	if (num_fences > index) {
> > > +		struct dma_fence **tmp;
> > > -		fences = nfences;
> > > +		/* Keep going even when reducing the size failed */
> > > +		tmp = krealloc_array(fences, index, sizeof(*fences),
> > > +				     GFP_KERNEL);
> > > +		if (tmp)
> > > +			fences = tmp;
> > >   	}
> > > -	if (sync_file_set_fence(sync_file, fences, i) < 0)
> > > -		goto err;
> > > +	if (sync_file_set_fence(sync_file, fences, index) < 0)
> > > +		goto err_put_fences;
> > >   	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
> > >   	return sync_file;
> > > -err:
> > > -	while (i)
> > > -		dma_fence_put(fences[--i]);
> > > +err_put_fences:
> > > +	while (index)
> > > +		dma_fence_put(fences[--index]);
> > >   	kfree(fences);
> > > +
> > > +err_free_sync_file:
> > >   	fput(sync_file->file);
> > >   	return NULL;
> > > -
> > >   }
> > >   static int sync_file_release(struct inode *inode, struct file *file)
> > > @@ -398,11 +396,13 @@ static int sync_fill_fence_info(struct dma_fence *fence,
> > >   static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> > >   				       unsigned long arg)
> > >   {
> > > -	struct sync_file_info info;
> > >   	struct sync_fence_info *fence_info = NULL;
> > > -	struct dma_fence **fences;
> > > +	struct dma_fence_unwrap iter;
> > > +	struct sync_file_info info;
> > > +	unsigned int num_fences;
> > > +	struct dma_fence *fence;
> > > +	int ret;
> > >   	__u32 size;
> > > -	int num_fences, ret, i;
> > >   	if (copy_from_user(&info, (void __user *)arg, sizeof(info)))
> > >   		return -EFAULT;
> > > @@ -410,7 +410,9 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> > >   	if (info.flags || info.pad)
> > >   		return -EINVAL;
> > > -	fences = get_fences(sync_file, &num_fences);
> > > +	num_fences = 0;
> > > +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence)
> > > +		++num_fences;
> > >   	/*
> > >   	 * Passing num_fences = 0 means that userspace doesn't want to
> > > @@ -433,8 +435,11 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> > >   	if (!fence_info)
> > >   		return -ENOMEM;
> > > -	for (i = 0; i < num_fences; i++) {
> > > -		int status = sync_fill_fence_info(fences[i], &fence_info[i]);
> > > +	num_fences = 0;
> > > +	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence) {
> > > +		int status;
> > > +
> > > +		status = sync_fill_fence_info(fence, &fence_info[num_fences++]);
> > >   		info.status = info.status <= 0 ? info.status : status;
> > >   	}
> > > -- 
> > > 2.25.1
> > > 
>

diff mbox series

Patch

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 394e6e1e9686..b8dea4ec123b 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -5,6 +5,7 @@ 
  * Copyright (C) 2012 Google, Inc.
  */
 
+#include <linux/dma-fence-unwrap.h>
 #include <linux/export.h>
 #include <linux/file.h>
 #include <linux/fs.h>
@@ -172,20 +173,6 @@  static int sync_file_set_fence(struct sync_file *sync_file,
 	return 0;
 }
 
-static struct dma_fence **get_fences(struct sync_file *sync_file,
-				     int *num_fences)
-{
-	if (dma_fence_is_array(sync_file->fence)) {
-		struct dma_fence_array *array = to_dma_fence_array(sync_file->fence);
-
-		*num_fences = array->num_fences;
-		return array->fences;
-	}
-
-	*num_fences = 1;
-	return &sync_file->fence;
-}
-
 static void add_fence(struct dma_fence **fences,
 		      int *i, struct dma_fence *fence)
 {
@@ -210,86 +197,97 @@  static void add_fence(struct dma_fence **fences,
 static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
 					 struct sync_file *b)
 {
+	struct dma_fence *a_fence, *b_fence, **fences;
+	struct dma_fence_unwrap a_iter, b_iter;
+	unsigned int index, num_fences;
 	struct sync_file *sync_file;
-	struct dma_fence **fences = NULL, **nfences, **a_fences, **b_fences;
-	int i = 0, i_a, i_b, num_fences, a_num_fences, b_num_fences;
 
 	sync_file = sync_file_alloc();
 	if (!sync_file)
 		return NULL;
 
-	a_fences = get_fences(a, &a_num_fences);
-	b_fences = get_fences(b, &b_num_fences);
-	if (a_num_fences > INT_MAX - b_num_fences)
-		goto err;
+	num_fences = 0;
+	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
+		++num_fences;
+	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
+		++num_fences;
 
-	num_fences = a_num_fences + b_num_fences;
+	if (num_fences > INT_MAX)
+		goto err_free_sync_file;
 
 	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
 	if (!fences)
-		goto err;
+		goto err_free_sync_file;
 
 	/*
-	 * Assume sync_file a and b are both ordered and have no
-	 * duplicates with the same context.
+	 * We can't guarantee that fences in both a and b are ordered, but it is
+	 * still quite likely.
 	 *
-	 * If a sync_file can only be created with sync_file_merge
-	 * and sync_file_create, this is a reasonable assumption.
+	 * So attempt to order the fences as we pass over them and merge fences
+	 * with the same context.
 	 */
-	for (i_a = i_b = 0; i_a < a_num_fences && i_b < b_num_fences; ) {
-		struct dma_fence *pt_a = a_fences[i_a];
-		struct dma_fence *pt_b = b_fences[i_b];
 
-		if (pt_a->context < pt_b->context) {
-			add_fence(fences, &i, pt_a);
+	index = 0;
+	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
+	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
+	     a_fence || b_fence; ) {
+
+		if (!b_fence) {
+			add_fence(fences, &index, a_fence);
+			a_fence = dma_fence_unwrap_next(&a_iter);
+
+		} else if (!a_fence) {
+			add_fence(fences, &index, b_fence);
+			b_fence = dma_fence_unwrap_next(&b_iter);
+
+		} else if (a_fence->context < b_fence->context) {
+			add_fence(fences, &index, a_fence);
+			a_fence = dma_fence_unwrap_next(&a_iter);
 
-			i_a++;
-		} else if (pt_a->context > pt_b->context) {
-			add_fence(fences, &i, pt_b);
+		} else if (b_fence->context < a_fence->context) {
+			add_fence(fences, &index, b_fence);
+			b_fence = dma_fence_unwrap_next(&b_iter);
+
+		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
+						a_fence->ops)) {
+			add_fence(fences, &index, a_fence);
+			a_fence = dma_fence_unwrap_next(&a_iter);
+			b_fence = dma_fence_unwrap_next(&b_iter);
 
-			i_b++;
 		} else {
-			if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno,
-						 pt_a->ops))
-				add_fence(fences, &i, pt_a);
-			else
-				add_fence(fences, &i, pt_b);
-
-			i_a++;
-			i_b++;
+			add_fence(fences, &index, b_fence);
+			a_fence = dma_fence_unwrap_next(&a_iter);
+			b_fence = dma_fence_unwrap_next(&b_iter);
 		}
 	}
 
-	for (; i_a < a_num_fences; i_a++)
-		add_fence(fences, &i, a_fences[i_a]);
-
-	for (; i_b < b_num_fences; i_b++)
-		add_fence(fences, &i, b_fences[i_b]);
-
-	if (i == 0)
-		fences[i++] = dma_fence_get(a_fences[0]);
+	if (index == 0)
+		add_fence(fences, &index, dma_fence_get_stub());
 
-	if (num_fences > i) {
-		nfences = krealloc_array(fences, i, sizeof(*fences), GFP_KERNEL);
-		if (!nfences)
-			goto err;
+	if (num_fences > index) {
+		struct dma_fence **tmp;
 
-		fences = nfences;
+		/* Keep going even when reducing the size failed */
+		tmp = krealloc_array(fences, index, sizeof(*fences),
+				     GFP_KERNEL);
+		if (tmp)
+			fences = tmp;
 	}
 
-	if (sync_file_set_fence(sync_file, fences, i) < 0)
-		goto err;
+	if (sync_file_set_fence(sync_file, fences, index) < 0)
+		goto err_put_fences;
 
 	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
 	return sync_file;
 
-err:
-	while (i)
-		dma_fence_put(fences[--i]);
+err_put_fences:
+	while (index)
+		dma_fence_put(fences[--index]);
 	kfree(fences);
+
+err_free_sync_file:
 	fput(sync_file->file);
 	return NULL;
-
 }
 
 static int sync_file_release(struct inode *inode, struct file *file)
@@ -398,11 +396,13 @@  static int sync_fill_fence_info(struct dma_fence *fence,
 static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
 				       unsigned long arg)
 {
-	struct sync_file_info info;
 	struct sync_fence_info *fence_info = NULL;
-	struct dma_fence **fences;
+	struct dma_fence_unwrap iter;
+	struct sync_file_info info;
+	unsigned int num_fences;
+	struct dma_fence *fence;
+	int ret;
 	__u32 size;
-	int num_fences, ret, i;
 
 	if (copy_from_user(&info, (void __user *)arg, sizeof(info)))
 		return -EFAULT;
@@ -410,7 +410,9 @@  static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
 	if (info.flags || info.pad)
 		return -EINVAL;
 
-	fences = get_fences(sync_file, &num_fences);
+	num_fences = 0;
+	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence)
+		++num_fences;
 
 	/*
 	 * Passing num_fences = 0 means that userspace doesn't want to
@@ -433,8 +435,11 @@  static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
 	if (!fence_info)
 		return -ENOMEM;
 
-	for (i = 0; i < num_fences; i++) {
-		int status = sync_fill_fence_info(fences[i], &fence_info[i]);
+	num_fences = 0;
+	dma_fence_unwrap_for_each(fence, &iter, sync_file->fence) {
+		int status;
+
+		status = sync_fill_fence_info(fence, &fence_info[num_fences++]);
 		info.status = info.status <= 0 ? info.status : status;
 	}