On Fri, Feb 28, 2020 at 01:24:02PM +0000, Jonathan Cameron wrote: > On Thu, 27 Feb 2020 06:58:11 +0000 > "Ardelean, Alexandru" <alexandru.Ardelean@xxxxxxxxxx> 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! Got it, Thanks > Thanks, > > Jonathan > > > > > > > > Thanks, > > > Rohit > >
