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:19 AM, 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.
I will add it.
> 
>>
>>>
>>>> +		fl->cctx->audio_init_mem = true;
> 
> What if there are two racing IOCTLs, trying to init AudioPD process?

There may be a chance that two threads enter ioctl at the same time, and 
seeing that audio_init_mem is false causes it to send twice, so a 
cctx->lock needs to be added here to ensure that two threads do not 
enter simultaneously.

> 
>>>> +		inbuf.pageslen = 1;
>>>> +	} else {
>>>> +		pages[0].addr = 0;
>>>> +		pages[0].size = 0;
>>>> +	}
>>>>    	args[2].ptr = (u64)(uintptr_t) pages;
>>>>    	args[2].length = sizeof(*pages);
>