Re: [PATCH v2] soc: qcom: pdr: Fix the potential deadlock

[Johan Hovold <johan@kernel.org>]

On Tue, Feb 11, 2025 at 10:37:11PM +0530, Mukesh Ojha wrote:
> On Mon, Feb 10, 2025 at 10:43:23AM +0100, Johan Hovold wrote:
> > On Mon, Feb 10, 2025 at 02:50:18PM +0530, Mukesh Ojha wrote:
> > > On Thu, Feb 06, 2025 at 04:13:25PM -0600, Bjorn Andersson wrote:

> > > > I came to the same patch while looking into the issue related to
> > > > in-kernel pd-mapper reported here:
> > > > https://lore.kernel.org/lkml/Zqet8iInnDhnxkT9@hovoldconsulting.com/
> > > > 
> > > > So:
> > > > Reviewed-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> > > > Tested-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 
> Should i add this in next version ?

Yes, if there is another revision.

> > I was gonna ask if you have confirmed that this indeed fixes the audio
> > regression with the in-kernel pd-mapper?
> > 
> > Is this how you discovered the issue as well, Mukesh and Saranya?
> 
> No, we are not using in kernel pd-mapper yet in downstream..

Ok, thanks for confirming.

> > If so, please mention that in the commit message, but in any case also
> > include the corresponding error messages directly so that people running
> > into this can find the fix more easily. (I see the pr_err now, but it's
> > not as greppable).
> 
> Below is the sample log which got in downstream when we hit this issue
> 
> 13.799119:   PDR: tms/servreg get domain list txn wait failed: -110
> 13.799146:   PDR: service lookup for msm/adsp/sensor_pd:tms/servreg failed: -110

I think it would be good to include this (without the time stamp) as an
example as it would make it easier to find this fix even if the failure
happens for another service.

> > A Link tag to my report would be good to have as well if this fixes the
> > audio regression.
> 
> I see this is somehow matching the logs you have reported, but this deadlock
> is there from the very first day of pdr_interface driver.
> 
> [   14.565059] PDR: avs/audio get domain list txn wait failed: -110
> [   14.571943] PDR: service lookup for avs/audio failed: -110

Yes, but using the in-kernel pd-mapper has exposed a number of existing
bugs since it changes the timing of events enough to make it easier to
hit them.

The audio regression is a very real regression for users of Snapdragon
based laptops like, for example, the Lenovo Yoga Slim 7x.

If Bjorn has confirmed that this is the same issue (I can try to
instrument the code based on your analysis to confirm this too), then I
think it would be good to mention this in the commit message and link to
the report, for example:

	This specifically also fixes an audio regression when using the
	in-kernel pd-mapper as that makes it easier to hit this race. [1]

	Link: https://lore.kernel.org/lkml/Zqet8iInnDhnxkT9@hovoldconsulting.com/ # [1]

or similar.

Johan