* query about locking in IIO
@ 2020-02-25 17:11 Rohit Sarkar
2020-02-26 6:54 ` Ardelean, Alexandru
0 siblings, 1 reply; 6+ messages in thread
From: Rohit Sarkar @ 2020-02-25 17:11 UTC (permalink / raw)
To: linux-iio; +Cc: rohitsarkar5398
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
Thanks,
Rohit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query about locking in IIO
2020-02-25 17:11 query about locking in IIO Rohit Sarkar
@ 2020-02-26 6:54 ` Ardelean, Alexandru
2020-02-26 17:25 ` Rohit Sarkar
0 siblings, 1 reply; 6+ messages in thread
From: Ardelean, Alexandru @ 2020-02-26 6:54 UTC (permalink / raw)
To: rohitsarkar5398@gmail.com, linux-iio@vger.kernel.org
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.
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query about locking in IIO
2020-02-26 6:54 ` Ardelean, Alexandru
@ 2020-02-26 17:25 ` Rohit Sarkar
2020-02-27 6:58 ` Ardelean, Alexandru
0 siblings, 1 reply; 6+ messages in thread
From: Rohit Sarkar @ 2020-02-26 17:25 UTC (permalink / raw)
To: Ardelean, Alexandru; +Cc: linux-iio@vger.kernel.org
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?
Thanks,
Rohit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query about locking in IIO
2020-02-26 17:25 ` Rohit Sarkar
@ 2020-02-27 6:58 ` Ardelean, Alexandru
2020-02-28 13:24 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ardelean, Alexandru @ 2020-02-27 6:58 UTC (permalink / raw)
To: rohitsarkar5398@gmail.com; +Cc: linux-iio@vger.kernel.org
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.
> Thanks,
> Rohit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query about locking in IIO
2020-02-27 6:58 ` Ardelean, Alexandru
@ 2020-02-28 13:24 ` Jonathan Cameron
2020-02-28 19:22 ` Rohit Sarkar
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2020-02-28 13:24 UTC (permalink / raw)
To: Ardelean, Alexandru; +Cc: rohitsarkar5398@gmail.com, linux-iio@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query about locking in IIO
2020-02-28 13:24 ` Jonathan Cameron
@ 2020-02-28 19:22 ` Rohit Sarkar
0 siblings, 0 replies; 6+ messages in thread
From: Rohit Sarkar @ 2020-02-28 19:22 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Ardelean, Alexandru, linux-iio@vger.kernel.org
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@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!
Got it, Thanks
> Thanks,
>
> Jonathan
>
> >
> >
> > > Thanks,
> > > Rohit
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-28 19:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-25 17:11 query about locking in IIO Rohit Sarkar
2020-02-26 6:54 ` Ardelean, Alexandru
2020-02-26 17:25 ` Rohit Sarkar
2020-02-27 6:58 ` Ardelean, Alexandru
2020-02-28 13:24 ` Jonathan Cameron
2020-02-28 19:22 ` Rohit Sarkar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).