Re: [PATCH v2] misc: fastrpc: Add support for multiple PD from one process

[Ekansh Gupta]

On 7/22/2024 1:48 PM, Dmitry Baryshkov wrote:
> On Sat, Jul 20, 2024 at 09:16:11AM GMT, 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 initmrequest, 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.
>>
>> Allocate and pass an effective pgid to DSP which would be allocated
> effective pgid makes me think about the setegid() system call. Can we
> just name them "client ID" (granted that session is already reserved)?
> Or is it really session ID? Can we use the index of the session instead
> and skip the whole IDR allocation?
Thanks for the suggestion, I'm trying out experiments with the index of session as
client ID to avoid idr operations. This sounds much better. I will update the patch
soon.

--Ekansh
>> during device open and will have a lifetime till the device is closed.
>> 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@xxxxxxxxxxx>
>> ---
>> Changes in v2:
>>   - Reformatted commit text.
>>   - Moved from ida to idr.
>>   - Changed dsp_pgid data type.
>>   - Resolved memory leak.
>>
>>  drivers/misc/fastrpc.c | 49 +++++++++++++++++++++++++++++++-----------
>>  1 file changed, 37 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a7a2bcedb37e..b4a5af2d2dfa 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -105,6 +105,10 @@
>>  
>>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>  
>> +#define MAX_DSP_PD	64	/* Maximum 64 PDs are allowed on DSP */
> Why?
>
>> +#define MIN_FRPC_PGID	1000
> Is it some random number or some pre-defined constant? Can we use 0
> instead?
>
>> +#define MAX_FRPC_PGID	(MIN_FRPC_PGID + MAX_DSP_PD)
>> +
>>  static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
>>  						"sdsp", "cdsp"};
>>  struct fastrpc_phy_page {
>