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 <[email protected]>
---
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 (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;
}
--
2.7.4
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/[email protected]/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 <[email protected]>
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/[email protected]
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/[email protected]/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 <[email protected]>
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/[email protected]
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 <[email protected]>
> ---
> 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;
> }
>
>