Re: query about locking in IIO

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 27 Feb 2020 06:58:11 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Wed, 2020-02-26 at 22:55 +0530, Rohit Sarkar wrote:
> > [External]
> > 
> > On Wed, Feb 26, 2020 at 06:54:21AM +0000, Ardelean, Alexandru wrote:  
> > > On Tue, 2020-02-25 at 22:41 +0530, Rohit Sarkar wrote:  
> > > > Hi,
> > > > Could someone explain why using indio_dev->mlock directly is a bad idea?
> > > > Further examples of cases where it cannot be replaced will be helpful
> > > >   
> > > 
> > > Jonathan may add more here.
> > > 
> > > But in general, each driver should define it's own explicit lock if it needs
> > > to.
> > > Some drivers need explicit locking, some don't.
> > > 
> > > A lot of other frameworks already define locks already.
> > > Like, for example, when an IIO driver uses some SPI transfers, the SPI
> > > framework
> > > already uses some locks. So, you don't typically need extra locking; which
> > > for
> > > some IIO drivers translates to: no extra explicit locking.
> > > 
> > > I guess Jonathan also wants to move the mlock to be used only in the IIO
> > > framework.
> > > In some cases, if drivers use this mlock, and the framework uses it, you can
> > > end
> > > up trying to acquire the same mlock twice, which can end-up in a deadlock.
> > > These things can sometimes slip through the code-review.  
> > 
> > This makes sense
> >   
> > > I guess the docs need a bit of update.
> > > Because:
> > > 
> > > * @mlock:              [DRIVER] lock used to prevent simultaneous device
> > > state
> > > *                      changes
> > > 
> > > I think it should be converted to [INTERN]
> > >   
> > > > Thanks,
> > > > Rohit
> > > >   
> > 
> > As a follow up would I be right to assume that as long as the mlock is
> > not being in the IIO framework, explicit locking should be the way to
> > go?  
> 
> The question sounds a bit hard to follow.
> Each driver should define it's own locking if it needs it.
> mlock will continued to be used only in the IIO framework; it won't be removed.
> [INTERN] is just a marker in the doc-string to make sure people don't use
> these fields in drivers.

Yes.  That's basically it.  mlock is a framework internal lock and we may change
how it is implemented at any time.

Various drivers using it make any such changes impossible and are much harder to
reason about because the mlock uses in the core aren't visible within the
driver.

So basically its a software architecture problem rather than there being any
known bugs!

Thanks,

Jonathan

> 
> 
> > Thanks,
> > Rohit