Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Jianping <jianping.li@oss.qualcomm.com>]

On 2/4/2026 5:42 AM, Bjorn Andersson wrote:
> On Tue, Feb 03, 2026 at 11:19:39PM +0200, Dmitry Baryshkov wrote:
>> On Mon, Feb 02, 2026 at 03:06:59PM +0800, Jianping wrote:
>>>
>>>
>>> On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
>>>> On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
>>>>> The entire reserved-memory region is now assigned to DSP VMIDs during
>>>>> channel setup and stored in cctx->remote_heap. Memory is reclaimed in
>>>>> rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
>>>>> heap lifecycle to the rpmsg channel.
>>>>
>>>>> @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>>>>    	args[1].length = inbuf.namelen;
>>>>>    	args[1].fd = -1;
>>>>> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>>> -	pages[0].size = fl->cctx->remote_heap->size;
>>>>> +	if (!fl->cctx->audio_init_mem) {
>>>>> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>>> +		pages[0].size = fl->cctx->remote_heap->size;
>>>>
>>>> Do we need a flag? Can't we assume that remote_heap is always to be
>>>> allocated to the PD?
>>> We do need the audio_init_mem flag.
>>> Once the PD starts and daemon takes the memory for the first time, PD will
>>> start using the memory,
>>> meanwhile, the daemon can be killed and restarted. In this case, the memory
>>> is still with the PD and the next
>>> daemon connection should not take any memory for the next request. This flag
>>> is maintained to ensure that.
>>> The memory needs to be resent only if Audio PD on DSP restarts(due to
>>> PD-restart or Subsystem-restart)
>>
>> This needs to be explained in the changelog.
>>
> 
> Not in the "changelog", in the commit message.
> 
> 
> @Jianping please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> 
> Your commit messages should "Describe your problem". From this
> description it should be clear why the change is needed and why the
> change is done in the specific way.
> 
> Regards,
> Bjorn

Thanks Bjorn for the reminder, I will revise my commit message.

Thanks,
Jianping

> 
>>>
>>>>
>>>>> +		fl->cctx->audio_init_mem = true;
>>
>> What if there are two racing IOCTLs, trying to init AudioPD process?
>>
>>>>> +		inbuf.pageslen = 1;
>>>>> +	} else {
>>>>> +		pages[0].addr = 0;
>>>>> +		pages[0].size = 0;
>>>>> +	}
>>>>>    	args[2].ptr = (u64)(uintptr_t) pages;
>>>>>    	args[2].length = sizeof(*pages);
>>
>> -- 
>> With best wishes
>> Dmitry
>>