Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jul 22, 2024 at 11:42:52AM +0530, Ekansh Gupta wrote:
> 
> 
> On 7/22/2024 11:28 AM, Greg KH wrote:
> > On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
> >> For user PD initialization, initmem is allocated and sent to DSP for
> >> initial memory requirements like shell loading. The size of this memory
> >> is decided based on the shell size that is passed by the user space.
> >> With the current implementation, a minimum of 2MB is always allocated
> >> for initmem even if the size passed by user is less than that. For this
> >> a MACRO is being used which is intended for shell size bound check.
> >> This minimum size of 2MB is not recommended as the PD will have very
> >> less memory for heap and will have to request HLOS again for memory.
> >> Define a new macro for initmem minimum length of 3MB.
> >>
> >> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
> >> Cc: stable <stable@kernel.org>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >>  drivers/misc/fastrpc.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> >> index a7a2bcedb37e..a3a5b745936e 100644
> >> --- a/drivers/misc/fastrpc.c
> >> +++ b/drivers/misc/fastrpc.c
> >> @@ -39,6 +39,7 @@
> >>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
> >>  #define FASTRPC_CTXID_MASK (0xFF0)
> >>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
> >> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
> > Meta-comment, for a future change, why not tabs to line things up?
> Sure, I'll add a comment.

I didn't say anything about comments :(

> Should I line up all the MACRO definitions? If yes, should I send it as a separate patch?

As I said, yes, for a future change.

> > How was this tested?
> This is tested with fastrpc use cases available in hexagon SDK:
> https://developer.qualcomm.com/software/hexagon-dsp-sdk/sample-apps

Do you have regression tests that attempt to check the boundry
conditions and alignment here?

thanks,

greg k-h