On 2/16/2026 9:26 AM, Bjorn Andersson wrote:
> 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"?
yes.
>
>> 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()?
The fdlist is updated by DSP. By "computation", I meant getting the location of
fdlist
in metadata buffer.
fastrpc_get_args allocates metadata buffer, fdlist is at some offset in this
buffer. This
list is updated by the DSP and rechecked during fastrpc_put_args for any
entries.
>
> PS. Use full function names and suffix them with (), to be clear in your
> description.
Ack.
>
>> 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.
I see the problem with the commit text. Let me come with a better description.
>
>> 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.
Ack.
>
>> 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?
DSP uses it for book-keeping mostly for maintaining DSP side mapping based on
fd.
Thanks for spending time on reviewing this change. I'll fix your concerns in
the next spin.
//Ekansh
>
> Regards,
> Bjorn
>
>> fastrpc_map_put(mmap);
>> }
>>
>> --
>> 2.34.1
>>
>>
