Re: [PATCH 06/14] ASoC: SOF: add a power status IPC

[Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>]

On Fri, Mar 20, 2020 at 02:27:32PM +0100, Guennadi Liakhovetski wrote:
> On Fri, Mar 20, 2020 at 12:19:52PM +0000, Mark Brown wrote:
> > On Fri, Mar 20, 2020 at 12:52:03PM +0100, Guennadi Liakhovetski wrote:
> > > On Fri, Mar 13, 2020 at 02:39:56PM +0000, Mark Brown wrote:
> > > > On Thu, Mar 12, 2020 at 03:44:21PM +0100, Guennadi Liakhovetski wrote:
> > 
> > > > >  #endif
> > > > > +	atomic_set(&sdev->reset_count, 0);
> > > > >  	dev_set_drvdata(dev, sdev);
> > 
> > > > Do we really need to use atomics for this?  They are hard to use
> > > > correctly.
> > 
> > > This variable is accessed from 2 contexts: it's incremented by the SOF 
> > > driver, when the firmware has booted and it's read by the SOF
> > > VirtIO backend vhost-be.c when receiving a resume request from the guest. 
> > > Timewise the variable will only be incremented during the DSP resume / 
> > > power up, while the VirtIO back end is waiting for the resume to complete in 
> > > pm_runtime_get_sync(). And only after that it reads the variable. But that 
> > > can happen on different CPUs. Whereas I think that runtime PM will sync 
> > > caches somewhere during the process, I think it is better to access the 
> > > variable in an SMP-safe way, e.g. using atomic operations.
> > 
> > That doesn't address my concern - to repeat, my concern is that atomics
> > are hard to use correctly.  Is there no other concurrency primitive (for
> > example this sounds like a completion) which can be used?
> 
> No, this isn't a completion - it's a counter. I've used atomic variables 
> before, I cannot remember seeing any difficulties with their correct use 
> described. Do you have a pointer?
> 
> Thinking about it, one problem I see is wrapping, it isn't currently 
> handled, but that would happen after quite a few PM suspend / resume 
> cycles... Still it can and should be fixed. But this isn't the concern, 
> that you have?

Actually I'd even say this isn't a problem. I think it's safe to say, you 
won't suspend and resume your audio interface more often than every 10 
seconds. That makes under 3200000 cycles per year. Even with 31 bits for a 
signed integer that makes more than 600 years. I think we're safe.

Thanks
Guennadi