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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <677db00b-117f-4732-b3ed-0a6714dab7cb@quicinc.com>
Date: Fri, 26 Jul 2024 12:06:58 +0530
From: Ekansh Gupta <quic_ekangupt@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <srinivas.kandagatla@...aro.org>, <linux-arm-msm@...r.kernel.org>,
        <gregkh@...uxfoundation.org>, <quic_bkumar@...cinc.com>,
        <linux-kernel@...r.kernel.org>, <quic_chennak@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <arnd@...db.de>
Subject: Re: [PATCH v2] misc: fastrpc: Add support for multiple PD from one
 process



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@...cinc.com>
>> ---
>> 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 {
>