| Message ID | 20220311110244.1245-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dma-buf: add dma_fence_unwrap | expand |
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
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
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
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 >
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 >>
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 --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; }
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(-)