Re: [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure

[Bjorn Andersson]

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 <ekansh.gupta@xxxxxxxxxxxxxxxx>
> ---
>  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
> 
>