| Message ID | 20240808104228.839629-1-quic_ekangupt@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v3] misc: fastrpc: Add support for multiple PD from one process | expand |
Hi Ekansh, On 08/08/2024 12:42, Ekansh Gupta wrote: > Memory intensive applications(which requires more tha 4GB) that wants > to offload tasks to DSP might have to split the tasks to multiple > user PD to make the resources available. > > For every call to DSP, fastrpc driver passes the process tgid which > works as an identifier for the DSP to enqueue the tasks to specific PD. > With current design, if any process opens device node more than once > and makes PD init request, same tgid will be passed to DSP which will > be considered a bad request and this will result in failure as the same > identifier cannot be used for multiple DSP PD. > > Assign and pass a client ID to DSP which would be assigned during device > open and will be dependent on the index of session allocated for the PD. > This will allow the same process to open the device more than once and > spawn multiple dynamic PD for ease of processing. A test tool to validate this fix and prevent it regressing in the future would be a good addition here. > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > --- > Changes in v2: > - Reformatted commit text. > - Moved from ida to idr. > - Changed dsp_pgid data type. > - Resolved memory leak. > Changes in v3: > - Modified commit text. > - Removed idr implementation. > - Using session index for client id. > > drivers/misc/fastrpc.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index a7a2bcedb37e..0ce1eedcb2c3 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -38,6 +38,7 @@ > #define FASTRPC_INIT_HANDLE 1 > #define FASTRPC_DSP_UTILITIES_HANDLE 2 > #define FASTRPC_CTXID_MASK (0xFF0) > +#define FASTRPC_CLIENTID_MASK (16) > #define INIT_FILELEN_MAX (2 * 1024 * 1024) > #define INIT_FILE_NAMELEN_MAX (128) > #define FASTRPC_DEVICE_NAME "fastrpc" > @@ -298,7 +299,7 @@ struct fastrpc_user { > struct fastrpc_session_ctx *sctx; > struct fastrpc_buf *init_mem; > > - int tgid; > + int client_id; > int pd; > bool is_secure_dev; > /* Lock for lists */ > @@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( > ctx->sc = sc; > ctx->retval = -1; > ctx->pid = current->pid; > - ctx->tgid = user->tgid; > + ctx->tgid = user->client_id; > ctx->cctx = cctx; > init_completion(&ctx->work); > INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); > @@ -1111,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, > int ret; > > cctx = fl->cctx; > - msg->pid = fl->tgid; > + msg->pid = fl->client_id; > msg->tid = current->pid; > > if (kernel) > @@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, > } > } > > - inbuf.pgid = fl->tgid; > + inbuf.pgid = fl->client_id; > inbuf.namelen = init.namelen; > inbuf.pageslen = 0; > fl->pd = USER_PD; > @@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > goto err; > } > > - inbuf.pgid = fl->tgid; > + inbuf.pgid = fl->client_id; > inbuf.namelen = strlen(current->comm) + 1; > inbuf.filelen = init.filelen; > inbuf.pageslen = 1; > @@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > } > > static struct fastrpc_session_ctx *fastrpc_session_alloc( > - struct fastrpc_channel_ctx *cctx) > + struct fastrpc_user *fl) > { > + struct fastrpc_channel_ctx *cctx = fl->cctx; > struct fastrpc_session_ctx *session = NULL; > unsigned long flags; > int i; > @@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc( > if (!cctx->session[i].used && cctx->session[i].valid) { > cctx->session[i].used = true; > session = &cctx->session[i]; > + fl->client_id = FASTRPC_CLIENTID_MASK | i; > break; > } > } > @@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) > int tgid = 0; > u32 sc; > > - tgid = fl->tgid; > + tgid = fl->client_id; > args[0].ptr = (u64)(uintptr_t) &tgid; > args[0].length = sizeof(tgid); > args[0].fd = -1; > @@ -1580,11 +1583,10 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) > INIT_LIST_HEAD(&fl->maps); > INIT_LIST_HEAD(&fl->mmaps); > INIT_LIST_HEAD(&fl->user); > - fl->tgid = current->tgid; > fl->cctx = cctx; > fl->is_secure_dev = fdevice->secure; > > - fl->sctx = fastrpc_session_alloc(cctx); > + fl->sctx = fastrpc_session_alloc(fl); > if (!fl->sctx) { > dev_err(&cctx->rpdev->dev, "No session available\n"); > mutex_destroy(&fl->mutex); > @@ -1648,7 +1650,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) > { > struct fastrpc_invoke_args args[1]; > - int tgid = fl->tgid; > + int tgid = fl->client_id; > u32 sc; > > args[0].ptr = (u64)(uintptr_t) &tgid; > @@ -1804,7 +1806,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * > int err; > u32 sc; > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.size = buf->size; > req_msg.vaddr = buf->raddr; > > @@ -1890,7 +1892,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > return err; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.flags = req.flags; > req_msg.vaddr = req.vaddrin; > req_msg.num = sizeof(pages); > @@ -1980,7 +1982,7 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me > return -EINVAL; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.len = map->len; > req_msg.vaddrin = map->raddr; > req_msg.fd = map->fd; > @@ -2033,7 +2035,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) > return err; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.fd = req.fd; > req_msg.offset = req.offset; > req_msg.vaddrin = req.vaddrin;
On 8/19/2024 4:35 PM, Caleb Connolly wrote: > Hi Ekansh, > > On 08/08/2024 12:42, Ekansh Gupta wrote: >> Memory intensive applications(which requires more tha 4GB) that wants >> to offload tasks to DSP might have to split the tasks to multiple >> user PD to make the resources available. >> >> For every call to DSP, fastrpc driver passes the process tgid which >> works as an identifier for the DSP to enqueue the tasks to specific PD. >> With current design, if any process opens device node more than once >> and makes PD init request, same tgid will be passed to DSP which will >> be considered a bad request and this will result in failure as the same >> identifier cannot be used for multiple DSP PD. >> >> Assign and pass a client ID to DSP which would be assigned during device >> open and will be dependent on the index of session allocated for the PD. >> This will allow the same process to open the device more than once and >> spawn multiple dynamic PD for ease of processing. > > A test tool to validate this fix and prevent it regressing in the future would be a good addition here. Thanks for reviewing the change, Caleb. This is more of a feature than a bug fix as it just adding support to spawn multiple user PDs from single process. Test cases for this feature was added to the recent versions of Hexagon SDK. --Ekansh >> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> >> --- >> Changes in v2: >> - Reformatted commit text. >> - Moved from ida to idr. >> - Changed dsp_pgid data type. >> - Resolved memory leak. >> Changes in v3: >> - Modified commit text. >> - Removed idr implementation. >> - Using session index for client id. >> >> drivers/misc/fastrpc.c | 30 ++++++++++++++++-------------- >> 1 file changed, 16 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> index a7a2bcedb37e..0ce1eedcb2c3 100644 >> --- a/drivers/misc/fastrpc.c >> +++ b/drivers/misc/fastrpc.c >> @@ -38,6 +38,7 @@ >> #define FASTRPC_INIT_HANDLE 1 >> #define FASTRPC_DSP_UTILITIES_HANDLE 2 >> #define FASTRPC_CTXID_MASK (0xFF0) >> +#define FASTRPC_CLIENTID_MASK (16) >> #define INIT_FILELEN_MAX (2 * 1024 * 1024) >> #define INIT_FILE_NAMELEN_MAX (128) >> #define FASTRPC_DEVICE_NAME "fastrpc" >> @@ -298,7 +299,7 @@ struct fastrpc_user { >> struct fastrpc_session_ctx *sctx; >> struct fastrpc_buf *init_mem; >> - int tgid; >> + int client_id; >> int pd; >> bool is_secure_dev; >> /* Lock for lists */ >> @@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( >> ctx->sc = sc; >> ctx->retval = -1; >> ctx->pid = current->pid; >> - ctx->tgid = user->tgid; >> + ctx->tgid = user->client_id; >> ctx->cctx = cctx; >> init_completion(&ctx->work); >> INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); >> @@ -1111,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, >> int ret; >> cctx = fl->cctx; >> - msg->pid = fl->tgid; >> + msg->pid = fl->client_id; >> msg->tid = current->pid; >> if (kernel) >> @@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, >> } >> } >> - inbuf.pgid = fl->tgid; >> + inbuf.pgid = fl->client_id; >> inbuf.namelen = init.namelen; >> inbuf.pageslen = 0; >> fl->pd = USER_PD; >> @@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, >> goto err; >> } >> - inbuf.pgid = fl->tgid; >> + inbuf.pgid = fl->client_id; >> inbuf.namelen = strlen(current->comm) + 1; >> inbuf.filelen = init.filelen; >> inbuf.pageslen = 1; >> @@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, >> } >> static struct fastrpc_session_ctx *fastrpc_session_alloc( >> - struct fastrpc_channel_ctx *cctx) >> + struct fastrpc_user *fl) >> { >> + struct fastrpc_channel_ctx *cctx = fl->cctx; >> struct fastrpc_session_ctx *session = NULL; >> unsigned long flags; >> int i; >> @@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc( >> if (!cctx->session[i].used && cctx->session[i].valid) { >> cctx->session[i].used = true; >> session = &cctx->session[i]; >> + fl->client_id = FASTRPC_CLIENTID_MASK | i; >> break; >> } >> } >> @@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) >> int tgid = 0; >> u32 sc; >> - tgid = fl->tgid; >> + tgid = fl->client_id; >> args[0].ptr = (u64)(uintptr_t) &tgid; >> args[0].length = sizeof(tgid); >> args[0].fd = -1; >> @@ -1580,11 +1583,10 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) >> INIT_LIST_HEAD(&fl->maps); >> INIT_LIST_HEAD(&fl->mmaps); >> INIT_LIST_HEAD(&fl->user); >> - fl->tgid = current->tgid; >> fl->cctx = cctx; >> fl->is_secure_dev = fdevice->secure; >> - fl->sctx = fastrpc_session_alloc(cctx); >> + fl->sctx = fastrpc_session_alloc(fl); >> if (!fl->sctx) { >> dev_err(&cctx->rpdev->dev, "No session available\n"); >> mutex_destroy(&fl->mutex); >> @@ -1648,7 +1650,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) >> static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) >> { >> struct fastrpc_invoke_args args[1]; >> - int tgid = fl->tgid; >> + int tgid = fl->client_id; >> u32 sc; >> args[0].ptr = (u64)(uintptr_t) &tgid; >> @@ -1804,7 +1806,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * >> int err; >> u32 sc; >> - req_msg.pgid = fl->tgid; >> + req_msg.pgid = fl->client_id; >> req_msg.size = buf->size; >> req_msg.vaddr = buf->raddr; >> @@ -1890,7 +1892,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) >> return err; >> } >> - req_msg.pgid = fl->tgid; >> + req_msg.pgid = fl->client_id; >> req_msg.flags = req.flags; >> req_msg.vaddr = req.vaddrin; >> req_msg.num = sizeof(pages); >> @@ -1980,7 +1982,7 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me >> return -EINVAL; >> } >> - req_msg.pgid = fl->tgid; >> + req_msg.pgid = fl->client_id; >> req_msg.len = map->len; >> req_msg.vaddrin = map->raddr; >> req_msg.fd = map->fd; >> @@ -2033,7 +2035,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) >> return err; >> } >> - req_msg.pgid = fl->tgid; >> + req_msg.pgid = fl->client_id; >> req_msg.fd = req.fd; >> req_msg.offset = req.offset; >> req_msg.vaddrin = req.vaddrin; >
On 20/08/2024 07:18, Ekansh Gupta wrote: > > > On 8/19/2024 4:35 PM, Caleb Connolly wrote: >> Hi Ekansh, >> >> On 08/08/2024 12:42, Ekansh Gupta wrote: >>> Memory intensive applications(which requires more tha 4GB) that wants >>> to offload tasks to DSP might have to split the tasks to multiple >>> user PD to make the resources available. >>> >>> For every call to DSP, fastrpc driver passes the process tgid which >>> works as an identifier for the DSP to enqueue the tasks to specific PD. >>> With current design, if any process opens device node more than once >>> and makes PD init request, same tgid will be passed to DSP which will >>> be considered a bad request and this will result in failure as the same >>> identifier cannot be used for multiple DSP PD. >>> >>> Assign and pass a client ID to DSP which would be assigned during device >>> open and will be dependent on the index of session allocated for the PD. >>> This will allow the same process to open the device more than once and >>> spawn multiple dynamic PD for ease of processing. >> >> A test tool to validate this fix and prevent it regressing in the future would be a good addition here. > Thanks for reviewing the change, Caleb. > > This is more of a feature than a bug fix as it just adding support to spawn multiple user PDs from > single process. Test cases for this feature was added to the recent versions of Hexagon SDK. The Hexagon SDK is not available without making an account on Qualcomm.com and clicking through at least on EULA. This should be publicly available source code (e.g. on GitHub) with documentation, something that someone with minimal experience using fastrpc could quickly get up and running on their device to test fastrpc. > > --Ekansh >>> >>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> >>> --- >>> Changes in v2: >>> - Reformatted commit text. >>> - Moved from ida to idr. >>> - Changed dsp_pgid data type. >>> - Resolved memory leak. >>> Changes in v3: >>> - Modified commit text. >>> - Removed idr implementation. >>> - Using session index for client id. >>> >>> drivers/misc/fastrpc.c | 30 ++++++++++++++++-------------- >>> 1 file changed, 16 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >>> index a7a2bcedb37e..0ce1eedcb2c3 100644 >>> --- a/drivers/misc/fastrpc.c >>> +++ b/drivers/misc/fastrpc.c >>> @@ -38,6 +38,7 @@ >>> #define FASTRPC_INIT_HANDLE 1 >>> #define FASTRPC_DSP_UTILITIES_HANDLE 2 >>> #define FASTRPC_CTXID_MASK (0xFF0) >>> +#define FASTRPC_CLIENTID_MASK (16) >>> #define INIT_FILELEN_MAX (2 * 1024 * 1024) >>> #define INIT_FILE_NAMELEN_MAX (128) >>> #define FASTRPC_DEVICE_NAME "fastrpc" >>> @@ -298,7 +299,7 @@ struct fastrpc_user { >>> struct fastrpc_session_ctx *sctx; >>> struct fastrpc_buf *init_mem; >>> - int tgid; >>> + int client_id; >>> int pd; >>> bool is_secure_dev; >>> /* Lock for lists */ >>> @@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( >>> ctx->sc = sc; >>> ctx->retval = -1; >>> ctx->pid = current->pid; >>> - ctx->tgid = user->tgid; >>> + ctx->tgid = user->client_id; >>> ctx->cctx = cctx; >>> init_completion(&ctx->work); >>> INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); >>> @@ -1111,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, >>> int ret; >>> cctx = fl->cctx; >>> - msg->pid = fl->tgid; >>> + msg->pid = fl->client_id; >>> msg->tid = current->pid; >>> if (kernel) >>> @@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, >>> } >>> } >>> - inbuf.pgid = fl->tgid; >>> + inbuf.pgid = fl->client_id; >>> inbuf.namelen = init.namelen; >>> inbuf.pageslen = 0; >>> fl->pd = USER_PD; >>> @@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, >>> goto err; >>> } >>> - inbuf.pgid = fl->tgid; >>> + inbuf.pgid = fl->client_id; >>> inbuf.namelen = strlen(current->comm) + 1; >>> inbuf.filelen = init.filelen; >>> inbuf.pageslen = 1; >>> @@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, >>> } >>> static struct fastrpc_session_ctx *fastrpc_session_alloc( >>> - struct fastrpc_channel_ctx *cctx) >>> + struct fastrpc_user *fl) >>> { >>> + struct fastrpc_channel_ctx *cctx = fl->cctx; >>> struct fastrpc_session_ctx *session = NULL; >>> unsigned long flags; >>> int i; >>> @@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc( >>> if (!cctx->session[i].used && cctx->session[i].valid) { >>> cctx->session[i].used = true; >>> session = &cctx->session[i]; >>> + fl->client_id = FASTRPC_CLIENTID_MASK | i; >>> break; >>> } >>> } >>> @@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) >>> int tgid = 0; >>> u32 sc; >>> - tgid = fl->tgid; >>> + tgid = fl->client_id; >>> args[0].ptr = (u64)(uintptr_t) &tgid; >>> args[0].length = sizeof(tgid); >>> args[0].fd = -1; >>> @@ -1580,11 +1583,10 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) >>> INIT_LIST_HEAD(&fl->maps); >>> INIT_LIST_HEAD(&fl->mmaps); >>> INIT_LIST_HEAD(&fl->user); >>> - fl->tgid = current->tgid; >>> fl->cctx = cctx; >>> fl->is_secure_dev = fdevice->secure; >>> - fl->sctx = fastrpc_session_alloc(cctx); >>> + fl->sctx = fastrpc_session_alloc(fl); >>> if (!fl->sctx) { >>> dev_err(&cctx->rpdev->dev, "No session available\n"); >>> mutex_destroy(&fl->mutex); >>> @@ -1648,7 +1650,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) >>> static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) >>> { >>> struct fastrpc_invoke_args args[1]; >>> - int tgid = fl->tgid; >>> + int tgid = fl->client_id; >>> u32 sc; >>> args[0].ptr = (u64)(uintptr_t) &tgid; >>> @@ -1804,7 +1806,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * >>> int err; >>> u32 sc; >>> - req_msg.pgid = fl->tgid; >>> + req_msg.pgid = fl->client_id; >>> req_msg.size = buf->size; >>> req_msg.vaddr = buf->raddr; >>> @@ -1890,7 +1892,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) >>> return err; >>> } >>> - req_msg.pgid = fl->tgid; >>> + req_msg.pgid = fl->client_id; >>> req_msg.flags = req.flags; >>> req_msg.vaddr = req.vaddrin; >>> req_msg.num = sizeof(pages); >>> @@ -1980,7 +1982,7 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me >>> return -EINVAL; >>> } >>> - req_msg.pgid = fl->tgid; >>> + req_msg.pgid = fl->client_id; >>> req_msg.len = map->len; >>> req_msg.vaddrin = map->raddr; >>> req_msg.fd = map->fd; >>> @@ -2033,7 +2035,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) >>> return err; >>> } >>> - req_msg.pgid = fl->tgid; >>> + req_msg.pgid = fl->client_id; >>> req_msg.fd = req.fd; >>> req_msg.offset = req.offset; >>> req_msg.vaddrin = req.vaddrin; >> >
Thanks Ekansh for the patch, On 08/08/2024 11:42, Ekansh Gupta wrote: > Memory intensive applications(which requires more tha 4GB) that wants > to offload tasks to DSP might have to split the tasks to multiple > user PD to make the resources available. > > For every call to DSP, fastrpc driver passes the process tgid which > works as an identifier for the DSP to enqueue the tasks to specific PD. > With current design, if any process opens device node more than once > and makes PD init request, same tgid will be passed to DSP which will > be considered a bad request and this will result in failure as the same > identifier cannot be used for multiple DSP PD. > > Assign and pass a client ID to DSP which would be assigned during device > open and will be dependent on the index of session allocated for the PD. > This will allow the same process to open the device more than once and > spawn multiple dynamic PD for ease of processing. > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > --- > Changes in v2: > - Reformatted commit text. > - Moved from ida to idr. > - Changed dsp_pgid data type. > - Resolved memory leak. > Changes in v3: > - Modified commit text. > - Removed idr implementation. > - Using session index for client id. > > drivers/misc/fastrpc.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index a7a2bcedb37e..0ce1eedcb2c3 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -38,6 +38,7 @@ > #define FASTRPC_INIT_HANDLE 1 > #define FASTRPC_DSP_UTILITIES_HANDLE 2 > #define FASTRPC_CTXID_MASK (0xFF0) > +#define FASTRPC_CLIENTID_MASK (16) MASK normally is a hex mask rather than integer. Looking at the actual code changes this looks like a flag rather than a mask. This is setting up bit 5 of the process group id in struct fastrpc_msg and other structs. This really needs some documentation of how the group id is partitioned to have these various fields. Something like: /* FastRPC Group ID fields*/ #define FASTRPC_GID_SESSION_ID_MASK GENMASK(3, 0) #define FASTRPC_GID_CLIENT_ID_MASK GENMASK(4, 4) > #define INIT_FILELEN_MAX (2 * 1024 * 1024) > #define INIT_FILE_NAMELEN_MAX (128) > #define FASTRPC_DEVICE_NAME "fastrpc" > @@ -298,7 +299,7 @@ struct fastrpc_user { > struct fastrpc_session_ctx *sctx; > struct fastrpc_buf *init_mem; > > - int tgid; > + int client_id; Can you we rename this as groupid, this clearly reflects the facts that group id has multiple things as described above. If group id is not the correct name instead it should be client_id, then this should also be reflected in various sturcts that have group id as tgid, pid, pgid an so.. that is another cleanup which should go as new patch. --srini > int pd; > bool is_secure_dev; > /* Lock for lists */ > @@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( > ctx->sc = sc; > ctx->retval = -1; > ctx->pid = current->pid; > - ctx->tgid = user->tgid; > + ctx->tgid = user->client_id; > ctx->cctx = cctx; > init_completion(&ctx->work); > INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); > @@ -1111,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, > int ret; > > cctx = fl->cctx; > - msg->pid = fl->tgid; > + msg->pid = fl->client_id; > msg->tid = current->pid; > > if (kernel) > @@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, > } > } > > - inbuf.pgid = fl->tgid; > + inbuf.pgid = fl->client_id; > inbuf.namelen = init.namelen; > inbuf.pageslen = 0; > fl->pd = USER_PD; > @@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > goto err; > } > > - inbuf.pgid = fl->tgid; > + inbuf.pgid = fl->client_id; > inbuf.namelen = strlen(current->comm) + 1; > inbuf.filelen = init.filelen; > inbuf.pageslen = 1; > @@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > } > > static struct fastrpc_session_ctx *fastrpc_session_alloc( > - struct fastrpc_channel_ctx *cctx) > + struct fastrpc_user *fl) > { > + struct fastrpc_channel_ctx *cctx = fl->cctx; > struct fastrpc_session_ctx *session = NULL; > unsigned long flags; > int i; > @@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc( > if (!cctx->session[i].used && cctx->session[i].valid) { > cctx->session[i].used = true; > session = &cctx->session[i]; > + fl->client_id = FASTRPC_CLIENTID_MASK | i; > break; > } > } > @@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) > int tgid = 0; > u32 sc; > > - tgid = fl->tgid; > + tgid = fl->client_id; > args[0].ptr = (u64)(uintptr_t) &tgid; > args[0].length = sizeof(tgid); > args[0].fd = -1; > @@ -1580,11 +1583,10 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) > INIT_LIST_HEAD(&fl->maps); > INIT_LIST_HEAD(&fl->mmaps); > INIT_LIST_HEAD(&fl->user); > - fl->tgid = current->tgid; > fl->cctx = cctx; > fl->is_secure_dev = fdevice->secure; > > - fl->sctx = fastrpc_session_alloc(cctx); > + fl->sctx = fastrpc_session_alloc(fl); > if (!fl->sctx) { > dev_err(&cctx->rpdev->dev, "No session available\n"); > mutex_destroy(&fl->mutex); > @@ -1648,7 +1650,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) > { > struct fastrpc_invoke_args args[1]; > - int tgid = fl->tgid; > + int tgid = fl->client_id; > u32 sc; > > args[0].ptr = (u64)(uintptr_t) &tgid; > @@ -1804,7 +1806,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * > int err; > u32 sc; > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.size = buf->size; > req_msg.vaddr = buf->raddr; > > @@ -1890,7 +1892,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > return err; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.flags = req.flags; > req_msg.vaddr = req.vaddrin; > req_msg.num = sizeof(pages); > @@ -1980,7 +1982,7 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me > return -EINVAL; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.len = map->len; > req_msg.vaddrin = map->raddr; > req_msg.fd = map->fd; > @@ -2033,7 +2035,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) > return err; > } > > - req_msg.pgid = fl->tgid; > + req_msg.pgid = fl->client_id; > req_msg.fd = req.fd; > req_msg.offset = req.offset; > req_msg.vaddrin = req.vaddrin;
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index a7a2bcedb37e..0ce1eedcb2c3 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -38,6 +38,7 @@ #define FASTRPC_INIT_HANDLE 1 #define FASTRPC_DSP_UTILITIES_HANDLE 2 #define FASTRPC_CTXID_MASK (0xFF0) +#define FASTRPC_CLIENTID_MASK (16) #define INIT_FILELEN_MAX (2 * 1024 * 1024) #define INIT_FILE_NAMELEN_MAX (128) #define FASTRPC_DEVICE_NAME "fastrpc" @@ -298,7 +299,7 @@ struct fastrpc_user { struct fastrpc_session_ctx *sctx; struct fastrpc_buf *init_mem; - int tgid; + int client_id; int pd; bool is_secure_dev; /* Lock for lists */ @@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( ctx->sc = sc; ctx->retval = -1; ctx->pid = current->pid; - ctx->tgid = user->tgid; + ctx->tgid = user->client_id; ctx->cctx = cctx; init_completion(&ctx->work); INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); @@ -1111,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, int ret; cctx = fl->cctx; - msg->pid = fl->tgid; + msg->pid = fl->client_id; msg->tid = current->pid; if (kernel) @@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, } } - inbuf.pgid = fl->tgid; + inbuf.pgid = fl->client_id; inbuf.namelen = init.namelen; inbuf.pageslen = 0; fl->pd = USER_PD; @@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, goto err; } - inbuf.pgid = fl->tgid; + inbuf.pgid = fl->client_id; inbuf.namelen = strlen(current->comm) + 1; inbuf.filelen = init.filelen; inbuf.pageslen = 1; @@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, } static struct fastrpc_session_ctx *fastrpc_session_alloc( - struct fastrpc_channel_ctx *cctx) + struct fastrpc_user *fl) { + struct fastrpc_channel_ctx *cctx = fl->cctx; struct fastrpc_session_ctx *session = NULL; unsigned long flags; int i; @@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc( if (!cctx->session[i].used && cctx->session[i].valid) { cctx->session[i].used = true; session = &cctx->session[i]; + fl->client_id = FASTRPC_CLIENTID_MASK | i; break; } } @@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) int tgid = 0; u32 sc; - tgid = fl->tgid; + tgid = fl->client_id; args[0].ptr = (u64)(uintptr_t) &tgid; args[0].length = sizeof(tgid); args[0].fd = -1; @@ -1580,11 +1583,10 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) INIT_LIST_HEAD(&fl->maps); INIT_LIST_HEAD(&fl->mmaps); INIT_LIST_HEAD(&fl->user); - fl->tgid = current->tgid; fl->cctx = cctx; fl->is_secure_dev = fdevice->secure; - fl->sctx = fastrpc_session_alloc(cctx); + fl->sctx = fastrpc_session_alloc(fl); if (!fl->sctx) { dev_err(&cctx->rpdev->dev, "No session available\n"); mutex_destroy(&fl->mutex); @@ -1648,7 +1650,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) { struct fastrpc_invoke_args args[1]; - int tgid = fl->tgid; + int tgid = fl->client_id; u32 sc; args[0].ptr = (u64)(uintptr_t) &tgid; @@ -1804,7 +1806,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * int err; u32 sc; - req_msg.pgid = fl->tgid; + req_msg.pgid = fl->client_id; req_msg.size = buf->size; req_msg.vaddr = buf->raddr; @@ -1890,7 +1892,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) return err; } - req_msg.pgid = fl->tgid; + req_msg.pgid = fl->client_id; req_msg.flags = req.flags; req_msg.vaddr = req.vaddrin; req_msg.num = sizeof(pages); @@ -1980,7 +1982,7 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me return -EINVAL; } - req_msg.pgid = fl->tgid; + req_msg.pgid = fl->client_id; req_msg.len = map->len; req_msg.vaddrin = map->raddr; req_msg.fd = map->fd; @@ -2033,7 +2035,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) return err; } - req_msg.pgid = fl->tgid; + req_msg.pgid = fl->client_id; req_msg.fd = req.fd; req_msg.offset = req.offset; req_msg.vaddrin = req.vaddrin;
Memory intensive applications(which requires more tha 4GB) that wants to offload tasks to DSP might have to split the tasks to multiple user PD to make the resources available. For every call to DSP, fastrpc driver passes the process tgid which works as an identifier for the DSP to enqueue the tasks to specific PD. With current design, if any process opens device node more than once and makes PD init request, same tgid will be passed to DSP which will be considered a bad request and this will result in failure as the same identifier cannot be used for multiple DSP PD. Assign and pass a client ID to DSP which would be assigned during device open and will be dependent on the index of session allocated for the PD. This will allow the same process to open the device more than once and spawn multiple dynamic PD for ease of processing. Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> --- Changes in v2: - Reformatted commit text. - Moved from ida to idr. - Changed dsp_pgid data type. - Resolved memory leak. Changes in v3: - Modified commit text. - Removed idr implementation. - Using session index for client id. drivers/misc/fastrpc.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)