On Sun, Feb 15, 2026 at 11:51:32PM +0530, Ekansh Gupta wrote: > The fdlist is currently part of the meta buffer,
Do you mean "already part of"? > computed during > put_args. The only "computation" I can see wrt fdlist in fastrpc_put_args() is where we calculate it to be at pages + inbufs + outbufs + handles. So, why do you say that the content of the meta buffer is calculated there? Did you mean fastrpc_get_args()? PS. Use full function names and suffix them with (), to be clear in your description. > This leads to code duplication when preparing and reading > critical meta buffer contents used by the FastRPC driver. "used by the whole entire FastRPC driver" is rather broad... As far as I can tell, the thing this patch is doing is caching the "fdlist" address, to avoid having to derive "pages" (and thereby indirectly "list"), "outbufs", "handles", and then sum these up. > > Move fdlist to the invoke context structure to improve maintainability > and reduce redundancy. This centralizes its handling and simplifies > meta buffer preparation and reading logic. I think the patch looks good, but your commit message is too high-level sales pitch. Describe the specific problem that you're solving (i.e. you're recalculating the offset which was known at the time of fastrpc_get_args()) and leave it at that. > > Signed-off-by: Ekansh Gupta <[email protected]> > --- > drivers/misc/fastrpc.c | 16 +++++----------- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 4f5a79c50f58..ce397c687161 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -233,6 +233,7 @@ struct fastrpc_invoke_ctx { > int pid; > int client_id; > u32 sc; > + u64 *fdlist; > u32 *crc; > u64 ctxid; > u64 msg_sz; > @@ -1018,6 +1019,7 @@ static int fastrpc_get_args(u32 kernel, struct > fastrpc_invoke_ctx *ctx) > rpra = ctx->buf->virt; > list = fastrpc_invoke_buf_start(rpra, ctx->nscalars); > pages = fastrpc_phy_page_start(list, ctx->nscalars); > + ctx->fdlist = (u64 *)(pages + ctx->nscalars); > args = (uintptr_t)ctx->buf->virt + metalen; > rlen = pkt_size - metalen; > ctx->rpra = rpra; > @@ -1120,18 +1122,10 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx > *ctx, > union fastrpc_remote_arg *rpra = ctx->rpra; > struct fastrpc_user *fl = ctx->fl; > struct fastrpc_map *mmap = NULL; > - struct fastrpc_invoke_buf *list; > - struct fastrpc_phy_page *pages; > - u64 *fdlist; > - int i, inbufs, outbufs, handles; > + int i, inbufs; > int ret = 0; > > inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); > - outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc); > - handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + > REMOTE_SCALARS_OUTHANDLES(ctx->sc); > - list = fastrpc_invoke_buf_start(rpra, ctx->nscalars); > - pages = fastrpc_phy_page_start(list, ctx->nscalars); > - fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); > > for (i = inbufs; i < ctx->nbufs; ++i) { > if (!ctx->maps[i]) { > @@ -1153,9 +1147,9 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx > *ctx, > cleanup_fdlist: > /* Clean up fdlist which is updated by DSP */ > for (i = 0; i < FASTRPC_MAX_FDLIST; i++) { > - if (!fdlist[i]) > + if (!ctx->fdlist[i]) It wouldn't hurt to keep the local fdlist variable, keeps the code less noisy - and you don't need to change these two places. > break; > - if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap)) > + if (!fastrpc_map_lookup(fl, (int)ctx->fdlist[i], &mmap)) Why are the fds stored as u64 in the metadata? Are the actual fd numbers somehow consumed by the DSP side? Regards, Bjorn > fastrpc_map_put(mmap); > } > > -- > 2.34.1 > >
