Message ID | 1638163720-23123-2-git-send-email-jeyr@codeaurora.org (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | Add DMA handle implementation | expand |
Hi Jeya, Thank you for the patch! Yet something to improve: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on v5.16-rc3 next-20211126] [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/Jeya-R/Add-DMA-handle-implementation/20211129-133228 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git cd455ebb748c4e198c8158e5d61b3034bf10f22b config: hexagon-randconfig-r045-20211129 (https://download.01.org/0day-ci/archive/20211129/202111291504.10gwO8AE-lkp@intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e) 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 # https://github.com/0day-ci/linux/commit/fda521c79abfc2f40115cb53cffb3a3886e8a2f9 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jeya-R/Add-DMA-handle-implementation/20211129-133228 git checkout fda521c79abfc2f40115cb53cffb3a3886e8a2f9 # 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=hexagon SHELL=/bin/bash drivers/misc/ 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 >>): >> drivers/misc/fastrpc.c:923:25: error: use of undeclared identifier 'fl' if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) ^ 1 error generated. vim +/fl +923 drivers/misc/fastrpc.c 886 887 static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, 888 u32 kernel) 889 { 890 struct fastrpc_remote_arg *rpra = ctx->rpra; 891 struct fastrpc_map *mmap = NULL; 892 struct fastrpc_invoke_buf *list; 893 struct fastrpc_phy_page *pages; 894 u64 *fdlist; 895 int i, inbufs, outbufs, handles; 896 897 inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); 898 outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc); 899 handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc); 900 list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); 901 pages = ctx->buf->virt + ctx->nscalars * (sizeof(*list) + 902 sizeof(*rpra)); 903 fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); 904 905 for (i = inbufs; i < ctx->nbufs; ++i) { 906 if (!ctx->maps[i]) { 907 void *src = (void *)(uintptr_t)rpra[i].pv; 908 void *dst = (void *)(uintptr_t)ctx->args[i].ptr; 909 u64 len = rpra[i].len; 910 911 if (!kernel) { 912 if (copy_to_user((void __user *)dst, src, len)) 913 return -EFAULT; 914 } else { 915 memcpy(dst, src, len); 916 } 917 } 918 } 919 920 for (i = 0; i < FASTRPC_MAX_FDLIST; i++) { 921 if (!fdlist[i]) 922 break; > 923 if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) 924 fastrpc_map_put(mmap); 925 } 926 927 return 0; 928 } 929 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Jeya, Thank you for the patch! Yet something to improve: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on v5.16-rc3 next-20211126] [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/Jeya-R/Add-DMA-handle-implementation/20211129-133228 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git cd455ebb748c4e198c8158e5d61b3034bf10f22b config: microblaze-randconfig-r031-20211128 (https://download.01.org/0day-ci/archive/20211129/202111291535.1s3d27nD-lkp@intel.com/config) compiler: microblaze-linux-gcc (GCC) 11.2.0 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 # https://github.com/0day-ci/linux/commit/fda521c79abfc2f40115cb53cffb3a3886e8a2f9 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jeya-R/Add-DMA-handle-implementation/20211129-133228 git checkout fda521c79abfc2f40115cb53cffb3a3886e8a2f9 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=microblaze SHELL=/bin/bash drivers/misc/ 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 >>): drivers/misc/fastrpc.c: In function 'fastrpc_put_args': >> drivers/misc/fastrpc.c:923:39: error: 'fl' undeclared (first use in this function); did you mean 'fd'? 923 | if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) | ^~ | fd drivers/misc/fastrpc.c:923:39: note: each undeclared identifier is reported only once for each function it appears in vim +923 drivers/misc/fastrpc.c 886 887 static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, 888 u32 kernel) 889 { 890 struct fastrpc_remote_arg *rpra = ctx->rpra; 891 struct fastrpc_map *mmap = NULL; 892 struct fastrpc_invoke_buf *list; 893 struct fastrpc_phy_page *pages; 894 u64 *fdlist; 895 int i, inbufs, outbufs, handles; 896 897 inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); 898 outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc); 899 handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc); 900 list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); 901 pages = ctx->buf->virt + ctx->nscalars * (sizeof(*list) + 902 sizeof(*rpra)); 903 fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); 904 905 for (i = inbufs; i < ctx->nbufs; ++i) { 906 if (!ctx->maps[i]) { 907 void *src = (void *)(uintptr_t)rpra[i].pv; 908 void *dst = (void *)(uintptr_t)ctx->args[i].ptr; 909 u64 len = rpra[i].len; 910 911 if (!kernel) { 912 if (copy_to_user((void __user *)dst, src, len)) 913 return -EFAULT; 914 } else { 915 memcpy(dst, src, len); 916 } 917 } 918 } 919 920 for (i = 0; i < FASTRPC_MAX_FDLIST; i++) { 921 if (!fdlist[i]) 922 break; > 923 if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) 924 fastrpc_map_put(mmap); 925 } 926 927 return 0; 928 } 929 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Thanks for the patch, On 29/11/2021 05:28, Jeya R wrote: > Add fdlist implementation to support dma handles. fdlist is populated > by DSP if any map is no longer used and it is freed during put_args. Does the dsp add all the fds (from in/out handles) to this list or only ones that are no-longer used by the dsp? > > Signed-off-by: Jeya R <jeyr@codeaurora.org> > --- > drivers/misc/fastrpc.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 39aca77..3c937ff 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -353,7 +353,7 @@ static void fastrpc_context_free(struct kref *ref) > ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount); > cctx = ctx->cctx; > > - for (i = 0; i < ctx->nscalars; i++) > + for (i = 0; i < ctx->nbufs; i++) > fastrpc_map_put(ctx->maps[i]); If above question is true, then who is going to free the rest of the scalars. > > if (ctx->buf) > @@ -785,6 +785,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx) > err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf); > if (err) > return err; > + memset(ctx->buf->virt, 0, pkt_size); Why do we need to make this zero, dma_alloc_coherent should have returned zeroed memory here anyway? > > rpra = ctx->buf->virt; > list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); > @@ -887,9 +888,19 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, > u32 kernel) > { > struct fastrpc_remote_arg *rpra = ctx->rpra; > - int i, inbufs; > + struct fastrpc_map *mmap = NULL; > + struct fastrpc_invoke_buf *list; > + struct fastrpc_phy_page *pages; > + u64 *fdlist; > + int i, inbufs, outbufs, handles; > > inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); > + outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc); > + handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc); > + list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); > + pages = ctx->buf->virt + ctx->nscalars * (sizeof(*list) + > + sizeof(*rpra)); > + fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); > > for (i = inbufs; i < ctx->nbufs; ++i) { > if (!ctx->maps[i]) { > @@ -906,6 +917,13 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, > } > } > > + for (i = 0; i < FASTRPC_MAX_FDLIST; i++) { > + if (!fdlist[i]) > + break; > + if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) fastrpc_map_find() is will invoke a kref_get on the map so calling single fastrpc_map_put() here is not going to work. driver will be leaking memory. Have you tested this patch with kmemleak enabled? --srini > + fastrpc_map_put(mmap); > + } > + > return 0; > } > >
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 39aca77..3c937ff 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -353,7 +353,7 @@ static void fastrpc_context_free(struct kref *ref) ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount); cctx = ctx->cctx; - for (i = 0; i < ctx->nscalars; i++) + for (i = 0; i < ctx->nbufs; i++) fastrpc_map_put(ctx->maps[i]); if (ctx->buf) @@ -785,6 +785,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx) err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf); if (err) return err; + memset(ctx->buf->virt, 0, pkt_size); rpra = ctx->buf->virt; list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); @@ -887,9 +888,19 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, u32 kernel) { struct fastrpc_remote_arg *rpra = ctx->rpra; - int i, inbufs; + struct fastrpc_map *mmap = NULL; + struct fastrpc_invoke_buf *list; + struct fastrpc_phy_page *pages; + u64 *fdlist; + int i, inbufs, outbufs, handles; inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); + outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc); + handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc); + list = ctx->buf->virt + ctx->nscalars * sizeof(*rpra); + pages = ctx->buf->virt + ctx->nscalars * (sizeof(*list) + + sizeof(*rpra)); + fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); for (i = inbufs; i < ctx->nbufs; ++i) { if (!ctx->maps[i]) { @@ -906,6 +917,13 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, } } + for (i = 0; i < FASTRPC_MAX_FDLIST; i++) { + if (!fdlist[i]) + break; + if (!fastrpc_map_find(fl, (int)fdlist[i], &mmap)) + fastrpc_map_put(mmap); + } + return 0; }
Add fdlist implementation to support dma handles. fdlist is populated by DSP if any map is no longer used and it is freed during put_args. Signed-off-by: Jeya R <jeyr@codeaurora.org> --- drivers/misc/fastrpc.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)