Re: [PATCH v1] misc: fastrpc: Free DMA handles for RPC calls with no arguments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/26/2023 3:57 PM, Srinivas Kandagatla wrote:
Thanks Ekansh for this patch.

few comments below

On 31/08/2023 07:23, Ekansh Gupta wrote:
The FDs for DMA handles to be freed is updated in fdlist by DSP over

So the dsp is updating the fd list after invoke?

Yes, correct. DSP updates this fd list when the buffer is no longer needed by the user PD.

a remote call. This holds true even for remote calls with no
arguments. To handle this, get_args and put_args are needed to
be called for remote calls with no arguments also as fdlist
is allocated in get_args and FDs updated in fdlist is freed
in put_args.

Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx>
---
  drivers/misc/fastrpc.c | 22 +++++++++-------------
  1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 9666d28..e6df66e 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1153,11 +1153,9 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
      if (IS_ERR(ctx))
          return PTR_ERR(ctx);
-    if (ctx->nscalars) {

Why do we need to remove this check?
fastrpc_internal_invoke will have nscalars set before calling. and we are not dealing with fdlist in fastrpc_get_args(), so am not sure what this change is helping with.
Memory for fdlist is allocated as part of fastrpc_get_args. The reason to add this change is that fdlist can be updated by DSP over a call with no remote arguments, for this call, there should be some fdlist allocated so the DSP can update the list if needed. Same apply for fastrpc_put_args also as when DSP updates fdlist for any remote call with no arguments, the maps corresponding to the fdlist should be freed.


-        err = fastrpc_get_args(kernel, ctx);
-        if (err)
-            goto bail;
-    }
+    err = fastrpc_get_args(kernel, ctx);
+    if (err)
+        goto bail;
      /* make sure that all CPU memory writes are seen by DSP */
      dma_wmb();
@@ -1181,14 +1179,12 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
      if (err)
          goto bail;
-    if (ctx->nscalars) {
-        /* make sure that all memory writes by DSP are seen by CPU */
-        dma_rmb();
-        /* populate all the output buffers with results */
-        err = fastrpc_put_args(ctx, kernel);
-        if (err)
-            goto bail;
-    }
+    /* make sure that all memory writes by DSP are seen by CPU */
+    dma_rmb();
+    /* populate all the output buffers with results */

A comment about fdlist here would be really useful
Sure, I will add a comment in the next patch. Do you suggest to add comment here or inside fastrpc_put_args function:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n1092

+    err = fastrpc_put_args(ctx, kernel);
+    if (err)
+        goto bail;
  bail:
      if (err != -ERESTARTSYS && err != -ETIMEDOUT) {



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux