* [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-11 15:13 ` Olivier Moysan
0 siblings, 0 replies; 11+ messages in thread
From: Olivier Moysan @ 2019-10-11 15:13 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, mcoquelin.stm32, alexandre.torgue,
fabrice.gasnier, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel, benjamin.gaignard, olivier.moysan
The aim of this patch is to correct a recursive locking warning,
detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
This message was initially triggered by the following call sequence
in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
in stm32_dfsdm_read_raw()
iio_device_claim_direct_mode
mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
iio_hw_consumer_enable
iio_update_buffers
mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
Here two instances of the same lock class are requested
on two different objects.
The locking validator needs to be informed of the nesting level
of each lock to avoid a false positive.
This patch introduces a class hierarchy in iio device lock,
assuming that hardware consumer is at a lower level than iio device.
[ 52.086174]
[ 52.086223] ============================================
[ 52.091516] WARNING: possible recursive locking detected
[ 52.096825] 4.19.49 #162 Not tainted
[ 52.100384] --------------------------------------------
[ 52.105691] cat/823 is trying to acquire lock:
[ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
[ 52.116995]
[ 52.116995] but task is already holding lock:
[ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
[ 52.130560]
[ 52.130560] other info that might help us debug this:
[ 52.137083] Possible unsafe locking scenario:
[ 52.137083]
[ 52.142995] CPU0
[ 52.145430] ----
[ 52.147864] lock(&dev->mlock);
[ 52.151082] lock(&dev->mlock);
[ 52.154301]
[ 52.154301] * DEADLOCK *
[ 52.154301]
[ 52.160215] May be due to missing lock nesting notation
[ 52.160215]
[ 52.167000] 5 locks held by cat/823:
[ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
[ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
[ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
[ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
[ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
[ 52.207431]
[ 52.207431] stack backtrace:
[ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
[ 52.217606] Hardware name: STM32 (Device Tree Support)
[ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
[ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
[ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
[ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
[ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
[ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
[ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
[ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
[ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
[ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
[ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
[ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
[ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
[ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
[ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
[ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
[ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
[ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
[ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
[ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
[ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
---
drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
drivers/iio/industrialio-buffer.c | 2 +-
drivers/iio/industrialio-core.c | 3 ++-
include/linux/iio/iio.h | 6 ++++++
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
index 95165697d8ae..652ce31b4b5f 100644
--- a/drivers/iio/buffer/industrialio-hw-consumer.c
+++ b/drivers/iio/buffer/industrialio-hw-consumer.c
@@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
chan = &hwc->channels[0];
while (chan->indio_dev) {
+ chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
if (!buf) {
ret = -ENOMEM;
@@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
{
struct hw_consumer_buffer *buf, *n;
+ struct iio_channel *chan = &hwc->channels[0];
+
+ while (chan->indio_dev) {
+ chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
+ iio_channel_release(chan);
+ chan++;
+ }
- iio_channel_release_all(hwc->channels);
list_for_each_entry_safe(buf, n, &hwc->buffers, head)
iio_buffer_put(&buf->buffer);
kfree(hwc);
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index c193d64e5217..d1df04167978 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
return 0;
mutex_lock(&indio_dev->info_exist_lock);
- mutex_lock(&indio_dev->mlock);
+ mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
if (insert_buffer && iio_buffer_is_active(insert_buffer))
insert_buffer = NULL;
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f72c2dc5f703..b14ba42559a3 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
dev->dev.groups = dev->groups;
dev->dev.type = &iio_device_type;
dev->dev.bus = &iio_bus_type;
+ dev->mutex_class = IIO_MUTEX_NORMAL;
device_initialize(&dev->dev);
dev_set_drvdata(&dev->dev, (void *)dev);
mutex_init(&dev->mlock);
@@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
*/
int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
{
- mutex_lock(&indio_dev->mlock);
+ mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
if (iio_buffer_enabled(indio_dev)) {
mutex_unlock(&indio_dev->mlock);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 862ce0019eba..1192eca124f4 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -17,6 +17,11 @@
* Currently assumes nano seconds.
*/
+enum iio_mutex_lock_class {
+ IIO_MUTEX_NORMAL,
+ IIO_MUTEX_HWC,
+};
+
enum iio_shared_by {
IIO_SEPARATE,
IIO_SHARED_BY_TYPE,
@@ -537,6 +542,7 @@ struct iio_dev {
struct list_head buffer_list;
int scan_bytes;
struct mutex mlock;
+ int mutex_class;
const unsigned long *available_scan_masks;
unsigned masklength;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-11 15:13 ` Olivier Moysan
0 siblings, 0 replies; 11+ messages in thread
From: Olivier Moysan @ 2019-10-11 15:13 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, mcoquelin.stm32, alexandre.torgue,
fabrice.gasnier, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel, benjamin.gaignard, olivier.moysan
The aim of this patch is to correct a recursive locking warning,
detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
This message was initially triggered by the following call sequence
in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
in stm32_dfsdm_read_raw()
iio_device_claim_direct_mode
mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
iio_hw_consumer_enable
iio_update_buffers
mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
Here two instances of the same lock class are requested
on two different objects.
The locking validator needs to be informed of the nesting level
of each lock to avoid a false positive.
This patch introduces a class hierarchy in iio device lock,
assuming that hardware consumer is at a lower level than iio device.
[ 52.086174]
[ 52.086223] ============================================
[ 52.091516] WARNING: possible recursive locking detected
[ 52.096825] 4.19.49 #162 Not tainted
[ 52.100384] --------------------------------------------
[ 52.105691] cat/823 is trying to acquire lock:
[ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
[ 52.116995]
[ 52.116995] but task is already holding lock:
[ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
[ 52.130560]
[ 52.130560] other info that might help us debug this:
[ 52.137083] Possible unsafe locking scenario:
[ 52.137083]
[ 52.142995] CPU0
[ 52.145430] ----
[ 52.147864] lock(&dev->mlock);
[ 52.151082] lock(&dev->mlock);
[ 52.154301]
[ 52.154301] * DEADLOCK *
[ 52.154301]
[ 52.160215] May be due to missing lock nesting notation
[ 52.160215]
[ 52.167000] 5 locks held by cat/823:
[ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
[ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
[ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
[ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
[ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
[ 52.207431]
[ 52.207431] stack backtrace:
[ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
[ 52.217606] Hardware name: STM32 (Device Tree Support)
[ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
[ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
[ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
[ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
[ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
[ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
[ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
[ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
[ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
[ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
[ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
[ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
[ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
[ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
[ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
[ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
[ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
[ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
[ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
[ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
[ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
---
drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
drivers/iio/industrialio-buffer.c | 2 +-
drivers/iio/industrialio-core.c | 3 ++-
include/linux/iio/iio.h | 6 ++++++
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
index 95165697d8ae..652ce31b4b5f 100644
--- a/drivers/iio/buffer/industrialio-hw-consumer.c
+++ b/drivers/iio/buffer/industrialio-hw-consumer.c
@@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
chan = &hwc->channels[0];
while (chan->indio_dev) {
+ chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
if (!buf) {
ret = -ENOMEM;
@@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
{
struct hw_consumer_buffer *buf, *n;
+ struct iio_channel *chan = &hwc->channels[0];
+
+ while (chan->indio_dev) {
+ chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
+ iio_channel_release(chan);
+ chan++;
+ }
- iio_channel_release_all(hwc->channels);
list_for_each_entry_safe(buf, n, &hwc->buffers, head)
iio_buffer_put(&buf->buffer);
kfree(hwc);
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index c193d64e5217..d1df04167978 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
return 0;
mutex_lock(&indio_dev->info_exist_lock);
- mutex_lock(&indio_dev->mlock);
+ mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
if (insert_buffer && iio_buffer_is_active(insert_buffer))
insert_buffer = NULL;
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f72c2dc5f703..b14ba42559a3 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
dev->dev.groups = dev->groups;
dev->dev.type = &iio_device_type;
dev->dev.bus = &iio_bus_type;
+ dev->mutex_class = IIO_MUTEX_NORMAL;
device_initialize(&dev->dev);
dev_set_drvdata(&dev->dev, (void *)dev);
mutex_init(&dev->mlock);
@@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
*/
int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
{
- mutex_lock(&indio_dev->mlock);
+ mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
if (iio_buffer_enabled(indio_dev)) {
mutex_unlock(&indio_dev->mlock);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 862ce0019eba..1192eca124f4 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -17,6 +17,11 @@
* Currently assumes nano seconds.
*/
+enum iio_mutex_lock_class {
+ IIO_MUTEX_NORMAL,
+ IIO_MUTEX_HWC,
+};
+
enum iio_shared_by {
IIO_SEPARATE,
IIO_SHARED_BY_TYPE,
@@ -537,6 +542,7 @@ struct iio_dev {
struct list_head buffer_list;
int scan_bytes;
struct mutex mlock;
+ int mutex_class;
const unsigned long *available_scan_masks;
unsigned masklength;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
2019-10-11 15:13 ` Olivier Moysan
@ 2019-10-12 8:57 ` Jonathan Cameron
-1 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2019-10-12 8:57 UTC (permalink / raw)
To: Olivier Moysan
Cc: knaack.h, lars, pmeerw, mcoquelin.stm32, alexandre.torgue,
fabrice.gasnier, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel, benjamin.gaignard
On Fri, 11 Oct 2019 17:13:14 +0200
Olivier Moysan <olivier.moysan@st.com> wrote:
> The aim of this patch is to correct a recursive locking warning,
> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
> This message was initially triggered by the following call sequence
> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>
> in stm32_dfsdm_read_raw()
> iio_device_claim_direct_mode
> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
> iio_hw_consumer_enable
> iio_update_buffers
> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
Hmm. I'm not sure I follow the logic. That lock is
for one thing and one thing only, preventing access
to the iio device that are unsafe when it is running
in a buffered mode. We shouldn't be in a position where
we both say don't do this if we are in buffered mode, + enter
buffered mode whilst doing this, or we need special functions
for entering buffering mode if in this state. We are in
some sense combining internal driver logic with overall
IIO states. IIO shouldn't care that the device is using
the same methods under the hood for buffered and non
buffered operations.
I can't really recall how this driver works. Is it actually
possible to have multiple hw_consumers at the same time?
So do we end up with multiple buffers registered and have
to demux out to the read_raw + the actual buffered path?
Given we have a bit of code saying grab one sample, I'm
going to guess we don't...
If so, the vast majority of the buffer setup code in IIO
is irrelevant here and we just need to call a few of
the callbacks from this driver directly... (I think
though I haven't chased through every corner.
I'd rather avoid introducing this nesting for a corner
case that makes no 'semantic' sense in IIO as it leaves us
in two separate states at the same time that the driver
is trying to make mutually exclusive. We can't both
not be in buffered mode, and in buffered mode.
Thanks and good luck with this nasty corner!
Jonathan
>
> Here two instances of the same lock class are requested
> on two different objects.
> The locking validator needs to be informed of the nesting level
> of each lock to avoid a false positive.
>
> This patch introduces a class hierarchy in iio device lock,
> assuming that hardware consumer is at a lower level than iio device.
>
> [ 52.086174]
> [ 52.086223] ============================================
> [ 52.091516] WARNING: possible recursive locking detected
> [ 52.096825] 4.19.49 #162 Not tainted
> [ 52.100384] --------------------------------------------
> [ 52.105691] cat/823 is trying to acquire lock:
> [ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
> [ 52.116995]
> [ 52.116995] but task is already holding lock:
> [ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
> [ 52.130560]
> [ 52.130560] other info that might help us debug this:
> [ 52.137083] Possible unsafe locking scenario:
> [ 52.137083]
> [ 52.142995] CPU0
> [ 52.145430] ----
> [ 52.147864] lock(&dev->mlock);
> [ 52.151082] lock(&dev->mlock);
> [ 52.154301]
> [ 52.154301] * DEADLOCK *
> [ 52.154301]
> [ 52.160215] May be due to missing lock nesting notation
> [ 52.160215]
> [ 52.167000] 5 locks held by cat/823:
> [ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
> [ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
> [ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
> [ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
> [ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
> [ 52.207431]
> [ 52.207431] stack backtrace:
> [ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
> [ 52.217606] Hardware name: STM32 (Device Tree Support)
> [ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
> [ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
> [ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
> [ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
> [ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
> [ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
> [ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
> [ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
> [ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
> [ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
> [ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
> [ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
> [ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
> [ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
> [ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
> [ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
> [ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
> [ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
> [ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
> [ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
> [ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
>
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> ---
> drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
> drivers/iio/industrialio-buffer.c | 2 +-
> drivers/iio/industrialio-core.c | 3 ++-
> include/linux/iio/iio.h | 6 ++++++
> 4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
> index 95165697d8ae..652ce31b4b5f 100644
> --- a/drivers/iio/buffer/industrialio-hw-consumer.c
> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
> @@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
>
> chan = &hwc->channels[0];
> while (chan->indio_dev) {
> + chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
> buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
> if (!buf) {
> ret = -ENOMEM;
> @@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
> void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
> {
> struct hw_consumer_buffer *buf, *n;
> + struct iio_channel *chan = &hwc->channels[0];
> +
> + while (chan->indio_dev) {
> + chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
> + iio_channel_release(chan);
> + chan++;
> + }
>
> - iio_channel_release_all(hwc->channels);
> list_for_each_entry_safe(buf, n, &hwc->buffers, head)
> iio_buffer_put(&buf->buffer);
> kfree(hwc);
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index c193d64e5217..d1df04167978 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
> return 0;
>
> mutex_lock(&indio_dev->info_exist_lock);
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>
> if (insert_buffer && iio_buffer_is_active(insert_buffer))
> insert_buffer = NULL;
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f72c2dc5f703..b14ba42559a3 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> dev->dev.groups = dev->groups;
> dev->dev.type = &iio_device_type;
> dev->dev.bus = &iio_bus_type;
> + dev->mutex_class = IIO_MUTEX_NORMAL;
> device_initialize(&dev->dev);
> dev_set_drvdata(&dev->dev, (void *)dev);
> mutex_init(&dev->mlock);
> @@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
> */
> int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
> {
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>
> if (iio_buffer_enabled(indio_dev)) {
> mutex_unlock(&indio_dev->mlock);
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 862ce0019eba..1192eca124f4 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -17,6 +17,11 @@
> * Currently assumes nano seconds.
> */
>
> +enum iio_mutex_lock_class {
> + IIO_MUTEX_NORMAL,
> + IIO_MUTEX_HWC,
> +};
> +
> enum iio_shared_by {
> IIO_SEPARATE,
> IIO_SHARED_BY_TYPE,
> @@ -537,6 +542,7 @@ struct iio_dev {
> struct list_head buffer_list;
> int scan_bytes;
> struct mutex mlock;
> + int mutex_class;
>
> const unsigned long *available_scan_masks;
> unsigned masklength;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-12 8:57 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2019-10-12 8:57 UTC (permalink / raw)
To: Olivier Moysan
Cc: lars, alexandre.torgue, linux-iio, pmeerw, linux-kernel,
mcoquelin.stm32, knaack.h, fabrice.gasnier, linux-stm32,
linux-arm-kernel, benjamin.gaignard
On Fri, 11 Oct 2019 17:13:14 +0200
Olivier Moysan <olivier.moysan@st.com> wrote:
> The aim of this patch is to correct a recursive locking warning,
> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
> This message was initially triggered by the following call sequence
> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>
> in stm32_dfsdm_read_raw()
> iio_device_claim_direct_mode
> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
> iio_hw_consumer_enable
> iio_update_buffers
> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
Hmm. I'm not sure I follow the logic. That lock is
for one thing and one thing only, preventing access
to the iio device that are unsafe when it is running
in a buffered mode. We shouldn't be in a position where
we both say don't do this if we are in buffered mode, + enter
buffered mode whilst doing this, or we need special functions
for entering buffering mode if in this state. We are in
some sense combining internal driver logic with overall
IIO states. IIO shouldn't care that the device is using
the same methods under the hood for buffered and non
buffered operations.
I can't really recall how this driver works. Is it actually
possible to have multiple hw_consumers at the same time?
So do we end up with multiple buffers registered and have
to demux out to the read_raw + the actual buffered path?
Given we have a bit of code saying grab one sample, I'm
going to guess we don't...
If so, the vast majority of the buffer setup code in IIO
is irrelevant here and we just need to call a few of
the callbacks from this driver directly... (I think
though I haven't chased through every corner.
I'd rather avoid introducing this nesting for a corner
case that makes no 'semantic' sense in IIO as it leaves us
in two separate states at the same time that the driver
is trying to make mutually exclusive. We can't both
not be in buffered mode, and in buffered mode.
Thanks and good luck with this nasty corner!
Jonathan
>
> Here two instances of the same lock class are requested
> on two different objects.
> The locking validator needs to be informed of the nesting level
> of each lock to avoid a false positive.
>
> This patch introduces a class hierarchy in iio device lock,
> assuming that hardware consumer is at a lower level than iio device.
>
> [ 52.086174]
> [ 52.086223] ============================================
> [ 52.091516] WARNING: possible recursive locking detected
> [ 52.096825] 4.19.49 #162 Not tainted
> [ 52.100384] --------------------------------------------
> [ 52.105691] cat/823 is trying to acquire lock:
> [ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
> [ 52.116995]
> [ 52.116995] but task is already holding lock:
> [ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
> [ 52.130560]
> [ 52.130560] other info that might help us debug this:
> [ 52.137083] Possible unsafe locking scenario:
> [ 52.137083]
> [ 52.142995] CPU0
> [ 52.145430] ----
> [ 52.147864] lock(&dev->mlock);
> [ 52.151082] lock(&dev->mlock);
> [ 52.154301]
> [ 52.154301] * DEADLOCK *
> [ 52.154301]
> [ 52.160215] May be due to missing lock nesting notation
> [ 52.160215]
> [ 52.167000] 5 locks held by cat/823:
> [ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
> [ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
> [ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
> [ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
> [ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
> [ 52.207431]
> [ 52.207431] stack backtrace:
> [ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
> [ 52.217606] Hardware name: STM32 (Device Tree Support)
> [ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
> [ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
> [ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
> [ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
> [ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
> [ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
> [ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
> [ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
> [ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
> [ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
> [ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
> [ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
> [ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
> [ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
> [ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
> [ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
> [ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
> [ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
> [ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
> [ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
> [ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
>
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> ---
> drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
> drivers/iio/industrialio-buffer.c | 2 +-
> drivers/iio/industrialio-core.c | 3 ++-
> include/linux/iio/iio.h | 6 ++++++
> 4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
> index 95165697d8ae..652ce31b4b5f 100644
> --- a/drivers/iio/buffer/industrialio-hw-consumer.c
> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
> @@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
>
> chan = &hwc->channels[0];
> while (chan->indio_dev) {
> + chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
> buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
> if (!buf) {
> ret = -ENOMEM;
> @@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
> void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
> {
> struct hw_consumer_buffer *buf, *n;
> + struct iio_channel *chan = &hwc->channels[0];
> +
> + while (chan->indio_dev) {
> + chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
> + iio_channel_release(chan);
> + chan++;
> + }
>
> - iio_channel_release_all(hwc->channels);
> list_for_each_entry_safe(buf, n, &hwc->buffers, head)
> iio_buffer_put(&buf->buffer);
> kfree(hwc);
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index c193d64e5217..d1df04167978 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
> return 0;
>
> mutex_lock(&indio_dev->info_exist_lock);
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>
> if (insert_buffer && iio_buffer_is_active(insert_buffer))
> insert_buffer = NULL;
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f72c2dc5f703..b14ba42559a3 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> dev->dev.groups = dev->groups;
> dev->dev.type = &iio_device_type;
> dev->dev.bus = &iio_bus_type;
> + dev->mutex_class = IIO_MUTEX_NORMAL;
> device_initialize(&dev->dev);
> dev_set_drvdata(&dev->dev, (void *)dev);
> mutex_init(&dev->mlock);
> @@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
> */
> int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
> {
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>
> if (iio_buffer_enabled(indio_dev)) {
> mutex_unlock(&indio_dev->mlock);
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 862ce0019eba..1192eca124f4 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -17,6 +17,11 @@
> * Currently assumes nano seconds.
> */
>
> +enum iio_mutex_lock_class {
> + IIO_MUTEX_NORMAL,
> + IIO_MUTEX_HWC,
> +};
> +
> enum iio_shared_by {
> IIO_SEPARATE,
> IIO_SHARED_BY_TYPE,
> @@ -537,6 +542,7 @@ struct iio_dev {
> struct list_head buffer_list;
> int scan_bytes;
> struct mutex mlock;
> + int mutex_class;
>
> const unsigned long *available_scan_masks;
> unsigned masklength;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
2019-10-11 15:13 ` Olivier Moysan
(?)
(?)
@ 2019-10-14 4:06 ` kbuild test robot
-1 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2019-10-14 4:06 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 25999 bytes --]
Hi Olivier,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on iio/togreg]
[cannot apply to v5.4-rc2 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Olivier-Moysan/iio-core-add-a-class-hierarchy-on-iio-device-lock/20191014-075428
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
reproduce: make htmldocs
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
WARNING: dot(1) not found, for better output quality install graphviz from http://www.graphviz.org
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
drivers/gpu/drm/mcde/mcde_drv.c:1: warning: 'ST-Ericsson MCDE DRM Driver' not found
include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
include/linux/spi/spi.h:190: warning: Function parameter or member 'driver_override' not described in 'spi_device'
include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops'
drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found
drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found
include/linux/w1.h:272: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
fs/direct-io.c:258: warning: Excess function parameter 'offset' description in 'dio_complete'
fs/libfs.c:496: warning: Excess function parameter 'available' description in 'simple_write_end'
fs/posix_acl.c:647: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
fs/posix_acl.c:647: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
fs/posix_acl.c:647: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
include/linux/i2c.h:337: warning: Function parameter or member 'init_irq' not described in 'i2c_client'
>> include/linux/iio/iio.h:578: warning: Function parameter or member 'mutex_class' not described in 'iio_dev'
lib/genalloc.c:1: warning: 'gen_pool_add_virt' not found
lib/genalloc.c:1: warning: 'gen_pool_alloc' not found
lib/genalloc.c:1: warning: 'gen_pool_free' not found
lib/genalloc.c:1: warning: 'gen_pool_alloc_algo' not found
mm/util.c:1: warning: 'get_user_pages_fast' not found
mm/slab.c:4215: warning: Function parameter or member 'objp' not described in '__ksize'
include/linux/skbuff.h:893: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'list' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
include/linux/skbuff.h:893: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
include/net/sock.h:233: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
include/net/sock.h:233: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
include/net/sock.h:515: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
include/net/sock.h:515: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
include/net/sock.h:2439: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE'
include/net/sock.h:2439: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE'
include/net/sock.h:2439: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE'
include/linux/netdevice.h:2040: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
include/linux/netdevice.h:2040: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
drivers/net/phy/phylink.c:595: warning: Function parameter or member 'config' not described in 'phylink_create'
drivers/net/phy/phylink.c:595: warning: Excess function parameter 'ndev' description in 'phylink_create'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:142: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:347: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:348: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:494: warning: Function parameter or member 'start' not described in 'amdgpu_vm_pt_first_dfs'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:823: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_flags'
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2822: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:378: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:379: warning: Function parameter or member 'ih' not described in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:379: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:1: warning: no structured comments found
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1: warning: no structured comments found
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1: warning: 'pp_dpm_sclk pp_dpm_mclk pp_dpm_pcie' not found
vim +578 include/linux/iio/iio.h
1612244f8a70da drivers/staging/iio/iio.h Jonathan Cameron 2011-12-05 487
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 488 /**
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 489 * struct iio_dev - industrial I/O device
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 490 * @id: [INTERN] used to identify device internally
63b19547cc3d96 include/linux/iio/iio.h Jonathan Cameron 2017-07-23 491 * @driver_module: [INTERN] used to make it harder to undercut users
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 492 * @modes: [DRIVER] operating modes supported by device
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 493 * @currentmode: [DRIVER] current operating mode
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 494 * @dev: [DRIVER] device structure, should be assigned a parent
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 495 * and owner
5aa9618896e0ba drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 496 * @event_interface: [INTERN] event chrdevs associated with interrupt lines
14555b14455f9a drivers/staging/iio/iio.h Jonathan Cameron 2011-09-21 497 * @buffer: [DRIVER] any buffer present
84b36ce5f79c01 include/linux/iio/iio.h Jonathan Cameron 2012-06-30 498 * @buffer_list: [INTERN] list of all buffers currently attached
420fe2e9471518 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 499 * @scan_bytes: [INTERN] num bytes captured to be fed to buffer demux
0118de7b4c04cb include/linux/iio/iio.h Daniel Baluta 2016-02-08 500 * @mlock: [DRIVER] lock used to prevent simultaneous device state
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 501 * changes
e5c003ae82865c drivers/staging/iio/iio.h Jonathan Cameron 2010-05-04 502 * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
32b5eecab0f2d6 drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 503 * @masklength: [INTERN] the length of the mask established from
32b5eecab0f2d6 drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 504 * channels
959d2952d124f0 drivers/staging/iio/iio.h Jonathan Cameron 2011-12-05 505 * @active_scan_mask: [INTERN] union of all scan masks requested by buffers
fd6487f8439f78 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 506 * @scan_timestamp: [INTERN] set if any buffers have requested timestamp
f1264809eb7fe4 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 507 * @scan_index_timestamp:[INTERN] cache of the index to the timestamp
14555b14455f9a drivers/staging/iio/iio.h Jonathan Cameron 2011-09-21 508 * @trig: [INTERN] current device trigger (buffer modes)
f00fd7ae4f409a include/linux/iio/iio.h Jonathan Corbet 2017-07-30 509 * @trig_readonly: [INTERN] mark the current trigger immutable
25985edcedea63 drivers/staging/iio/iio.h Lucas De Marchi 2011-03-30 510 * @pollfunc: [DRIVER] function run on trigger being received
735ad074ffa72c include/linux/iio/iio.h Vladimir Barinov 2015-08-20 511 * @pollfunc_event: [DRIVER] function run on events trigger being received
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 512 * @channels: [DRIVER] channel specification structure table
1787948873fd2c include/linux/iio/iio.h Peter Meerwald 2012-06-12 513 * @num_channels: [DRIVER] number of channels specified in @channels.
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 514 * @channel_attr_list: [INTERN] keep track of automatically created channel
1a25e5928e5ff7 drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 515 * attributes
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 516 * @chan_attr_group: [INTERN] group for all attrs in base directory
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 517 * @name: [DRIVER] name of the device.
2c3d0c9ffd24d9 include/linux/iio/iio.h Phil Reid 2019-09-19 518 * @label: [DRIVER] unique name to identify which device this is
1a25e5928e5ff7 drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 519 * @info: [DRIVER] callbacks and constant info from driver
bc2b7dab629a51 include/linux/iio/iio.h Gregor Boirie 2016-03-09 520 * @clock_id: [INTERN] timestamping clock posix identifier
ac917a81117ce0 drivers/staging/iio/iio.h Jonathan Cameron 2012-02-15 521 * @info_exist_lock: [INTERN] lock to prevent use during removal
ecbf20ca95546f drivers/staging/iio/iio.h Jonathan Cameron 2012-04-10 522 * @setup_ops: [DRIVER] callbacks to call before and after buffer
ecbf20ca95546f drivers/staging/iio/iio.h Jonathan Cameron 2012-04-10 523 * enable/disable
1a25e5928e5ff7 drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 524 * @chrdev: [INTERN] associated character device
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 525 * @groups: [INTERN] attribute groups
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 526 * @groupcounter: [INTERN] index of next attribute group
bb01443e2cdad4 drivers/staging/iio/iio.h Lars-Peter Clausen 2011-12-19 527 * @flags: [INTERN] file ops related flags including busy flag.
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 528 * @debugfs_dentry: [INTERN] device specific debugfs dentry.
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 529 * @cached_reg_addr: [INTERN] cached register address for debugfs reads.
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 530 */
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 531 struct iio_dev {
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 532 int id;
63b19547cc3d96 include/linux/iio/iio.h Jonathan Cameron 2017-07-23 533 struct module *driver_module;
4024bc73b1d1d8 drivers/staging/iio/iio.h Jonathan Cameron 2011-08-12 534
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 535 int modes;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 536 int currentmode;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 537 struct device dev;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 538
5aa9618896e0ba drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 539 struct iio_event_interface *event_interface;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 540
14555b14455f9a drivers/staging/iio/iio.h Jonathan Cameron 2011-09-21 541 struct iio_buffer *buffer;
84b36ce5f79c01 include/linux/iio/iio.h Jonathan Cameron 2012-06-30 542 struct list_head buffer_list;
420fe2e9471518 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 543 int scan_bytes;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 544 struct mutex mlock;
dc21ae17d2148d include/linux/iio/iio.h Olivier Moysan 2019-10-11 545 int mutex_class;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 546
cd4361c7e2e077 drivers/staging/iio/iio.h Michael Hennerich 2012-02-22 547 const unsigned long *available_scan_masks;
32b5eecab0f2d6 drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 548 unsigned masklength;
cd4361c7e2e077 drivers/staging/iio/iio.h Michael Hennerich 2012-02-22 549 const unsigned long *active_scan_mask;
fd6487f8439f78 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 550 bool scan_timestamp;
f1264809eb7fe4 drivers/staging/iio/iio.h Jonathan Cameron 2012-04-21 551 unsigned scan_index_timestamp;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 552 struct iio_trigger *trig;
c8cdf70890d89c include/linux/iio/iio.h Matt Ranostay 2016-09-02 553 bool trig_readonly;
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 554 struct iio_poll_func *pollfunc;
735ad074ffa72c include/linux/iio/iio.h Vladimir Barinov 2015-08-20 555 struct iio_poll_func *pollfunc_event;
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 556
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 557 struct iio_chan_spec const *channels;
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 558 int num_channels;
1d892719e70e47 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 559
6fe8135fccd66a drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 560 struct list_head channel_attr_list;
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 561 struct attribute_group chan_attr_group;
1b732888d83f71 drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 562 const char *name;
2c3d0c9ffd24d9 include/linux/iio/iio.h Phil Reid 2019-09-19 563 const char *label;
6fe8135fccd66a drivers/staging/iio/iio.h Jonathan Cameron 2011-05-18 564 const struct iio_info *info;
bc2b7dab629a51 include/linux/iio/iio.h Gregor Boirie 2016-03-09 565 clockid_t clock_id;
ac917a81117ce0 drivers/staging/iio/iio.h Jonathan Cameron 2012-02-15 566 struct mutex info_exist_lock;
1612244f8a70da drivers/staging/iio/iio.h Jonathan Cameron 2011-12-05 567 const struct iio_buffer_setup_ops *setup_ops;
1aa042783251c2 drivers/staging/iio/iio.h Jonathan Cameron 2011-08-30 568 struct cdev chrdev;
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 569 #define IIO_MAX_GROUPS 6
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 570 const struct attribute_group *groups[IIO_MAX_GROUPS + 1];
26d25ae3f0d8ff drivers/staging/iio/iio.h Jonathan Cameron 2011-09-02 571 int groupcounter;
bb01443e2cdad4 drivers/staging/iio/iio.h Lars-Peter Clausen 2011-12-19 572
bb01443e2cdad4 drivers/staging/iio/iio.h Lars-Peter Clausen 2011-12-19 573 unsigned long flags;
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 574 #if defined(CONFIG_DEBUG_FS)
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 575 struct dentry *debugfs_dentry;
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 576 unsigned cached_reg_addr;
e553f182d55bd2 drivers/staging/iio/iio.h Michael Hennerich 2012-03-01 577 #endif
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 @578 };
847ec80bbaa76a drivers/staging/iio/iio.h Jonathan Cameron 2009-08-18 579
:::::: The code at line 578 was first introduced by commit
:::::: 847ec80bbaa76aae41062d6802cea9c1b2289f14 Staging: IIO: core support for device registration and management
:::::: TO: Jonathan Cameron <jic23@cam.ac.uk>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 7282 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
2019-10-12 8:57 ` Jonathan Cameron
@ 2019-10-14 15:59 ` Olivier MOYSAN
-1 siblings, 0 replies; 11+ messages in thread
From: Olivier MOYSAN @ 2019-10-14 15:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: knaack.h, lars, pmeerw, mcoquelin.stm32, Alexandre TORGUE,
Fabrice GASNIER, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel, Benjamin GAIGNARD
Hello Jonathan,
Thanks for your comment.
On 10/12/19 10:57 AM, Jonathan Cameron wrote:
> On Fri, 11 Oct 2019 17:13:14 +0200
> Olivier Moysan <olivier.moysan@st.com> wrote:
>
>> The aim of this patch is to correct a recursive locking warning,
>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
>> This message was initially triggered by the following call sequence
>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>>
>> in stm32_dfsdm_read_raw()
>> iio_device_claim_direct_mode
>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
>> iio_hw_consumer_enable
>> iio_update_buffers
>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
> Hmm. I'm not sure I follow the logic. That lock is
> for one thing and one thing only, preventing access
> to the iio device that are unsafe when it is running
> in a buffered mode. We shouldn't be in a position where
> we both say don't do this if we are in buffered mode, + enter
> buffered mode whilst doing this, or we need special functions
> for entering buffering mode if in this state. We are in
> some sense combining internal driver logic with overall
> IIO states. IIO shouldn't care that the device is using
> the same methods under the hood for buffered and non
> buffered operations.
>
> I can't really recall how this driver works. Is it actually
> possible to have multiple hw_consumers at the same time?
>
> So do we end up with multiple buffers registered and have
> to demux out to the read_raw + the actual buffered path?
> Given we have a bit of code saying grab one sample, I'm
> going to guess we don't...
>
> If so, the vast majority of the buffer setup code in IIO
> is irrelevant here and we just need to call a few of
> the callbacks from this driver directly... (I think
> though I haven't chased through every corner.
>
> I'd rather avoid introducing this nesting for a corner
> case that makes no 'semantic' sense in IIO as it leaves us
> in two separate states at the same time that the driver
> is trying to make mutually exclusive. We can't both
> not be in buffered mode, and in buffered mode.
>
> Thanks and good luck with this nasty corner!
>
> Jonathan
>
Here I consider the following use case:
A single conversion is performed. The dfsdm (filter) is chained with a
front-end, which can be an ADC or a sensor. So we have two IIO devices,
the dfsdm and its front-end handled through the hw consumer interface.
You are right. There is something wrong here, in buffered/non-buffered
mode mixing.
iio_hw_consumer_enable() call is used to enable the front-end device.
But this interface is intended for buffered mode.
So this is not coherent with the expected single conversion mode,
indeed. Another interface is required to manage the front-end device. I
have a poor knowledge of iio framework, but it seems to me that there is
no interface to manage this.
My understanding regarding mlock, is that it is used to protect the
state of the iio device.
I we want to do a conversion from the chained devices, I think we need
to activate the first device
and keep it performing conversion, as long as the second device has done
its conversion.
We need to protect both devices, and we should have to do it in a nested
way.
So, I guess that anyway, nested mutexes would be required in this case.
Best regards
Olivier
>
>> Here two instances of the same lock class are requested
>> on two different objects.
>> The locking validator needs to be informed of the nesting level
>> of each lock to avoid a false positive.
>>
>> This patch introduces a class hierarchy in iio device lock,
>> assuming that hardware consumer is at a lower level than iio device.
>>
>> [ 52.086174]
>> [ 52.086223] ============================================
>> [ 52.091516] WARNING: possible recursive locking detected
>> [ 52.096825] 4.19.49 #162 Not tainted
>> [ 52.100384] --------------------------------------------
>> [ 52.105691] cat/823 is trying to acquire lock:
>> [ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
>> [ 52.116995]
>> [ 52.116995] but task is already holding lock:
>> [ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
>> [ 52.130560]
>> [ 52.130560] other info that might help us debug this:
>> [ 52.137083] Possible unsafe locking scenario:
>> [ 52.137083]
>> [ 52.142995] CPU0
>> [ 52.145430] ----
>> [ 52.147864] lock(&dev->mlock);
>> [ 52.151082] lock(&dev->mlock);
>> [ 52.154301]
>> [ 52.154301] * DEADLOCK *
>> [ 52.154301]
>> [ 52.160215] May be due to missing lock nesting notation
>> [ 52.160215]
>> [ 52.167000] 5 locks held by cat/823:
>> [ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
>> [ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
>> [ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
>> [ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
>> [ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
>> [ 52.207431]
>> [ 52.207431] stack backtrace:
>> [ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
>> [ 52.217606] Hardware name: STM32 (Device Tree Support)
>> [ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
>> [ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
>> [ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
>> [ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
>> [ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
>> [ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
>> [ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
>> [ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
>> [ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
>> [ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
>> [ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
>> [ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
>> [ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
>> [ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
>> [ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
>> [ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
>> [ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
>> [ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
>> [ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
>> [ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
>> [ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
>> ---
>> drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
>> drivers/iio/industrialio-buffer.c | 2 +-
>> drivers/iio/industrialio-core.c | 3 ++-
>> include/linux/iio/iio.h | 6 ++++++
>> 4 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
>> index 95165697d8ae..652ce31b4b5f 100644
>> --- a/drivers/iio/buffer/industrialio-hw-consumer.c
>> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
>> @@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
>>
>> chan = &hwc->channels[0];
>> while (chan->indio_dev) {
>> + chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
>> buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
>> if (!buf) {
>> ret = -ENOMEM;
>> @@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
>> void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
>> {
>> struct hw_consumer_buffer *buf, *n;
>> + struct iio_channel *chan = &hwc->channels[0];
>> +
>> + while (chan->indio_dev) {
>> + chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
>> + iio_channel_release(chan);
>> + chan++;
>> + }
>>
>> - iio_channel_release_all(hwc->channels);
>> list_for_each_entry_safe(buf, n, &hwc->buffers, head)
>> iio_buffer_put(&buf->buffer);
>> kfree(hwc);
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index c193d64e5217..d1df04167978 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
>> return 0;
>>
>> mutex_lock(&indio_dev->info_exist_lock);
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>>
>> if (insert_buffer && iio_buffer_is_active(insert_buffer))
>> insert_buffer = NULL;
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index f72c2dc5f703..b14ba42559a3 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>> dev->dev.groups = dev->groups;
>> dev->dev.type = &iio_device_type;
>> dev->dev.bus = &iio_bus_type;
>> + dev->mutex_class = IIO_MUTEX_NORMAL;
>> device_initialize(&dev->dev);
>> dev_set_drvdata(&dev->dev, (void *)dev);
>> mutex_init(&dev->mlock);
>> @@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
>> */
>> int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
>> {
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>>
>> if (iio_buffer_enabled(indio_dev)) {
>> mutex_unlock(&indio_dev->mlock);
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index 862ce0019eba..1192eca124f4 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -17,6 +17,11 @@
>> * Currently assumes nano seconds.
>> */
>>
>> +enum iio_mutex_lock_class {
>> + IIO_MUTEX_NORMAL,
>> + IIO_MUTEX_HWC,
>> +};
>> +
>> enum iio_shared_by {
>> IIO_SEPARATE,
>> IIO_SHARED_BY_TYPE,
>> @@ -537,6 +542,7 @@ struct iio_dev {
>> struct list_head buffer_list;
>> int scan_bytes;
>> struct mutex mlock;
>> + int mutex_class;
>>
>> const unsigned long *available_scan_masks;
>> unsigned masklength;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-14 15:59 ` Olivier MOYSAN
0 siblings, 0 replies; 11+ messages in thread
From: Olivier MOYSAN @ 2019-10-14 15:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: lars, Alexandre TORGUE, linux-iio, pmeerw, linux-kernel,
mcoquelin.stm32, knaack.h, Fabrice GASNIER, linux-stm32,
linux-arm-kernel, Benjamin GAIGNARD
Hello Jonathan,
Thanks for your comment.
On 10/12/19 10:57 AM, Jonathan Cameron wrote:
> On Fri, 11 Oct 2019 17:13:14 +0200
> Olivier Moysan <olivier.moysan@st.com> wrote:
>
>> The aim of this patch is to correct a recursive locking warning,
>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
>> This message was initially triggered by the following call sequence
>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>>
>> in stm32_dfsdm_read_raw()
>> iio_device_claim_direct_mode
>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
>> iio_hw_consumer_enable
>> iio_update_buffers
>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
> Hmm. I'm not sure I follow the logic. That lock is
> for one thing and one thing only, preventing access
> to the iio device that are unsafe when it is running
> in a buffered mode. We shouldn't be in a position where
> we both say don't do this if we are in buffered mode, + enter
> buffered mode whilst doing this, or we need special functions
> for entering buffering mode if in this state. We are in
> some sense combining internal driver logic with overall
> IIO states. IIO shouldn't care that the device is using
> the same methods under the hood for buffered and non
> buffered operations.
>
> I can't really recall how this driver works. Is it actually
> possible to have multiple hw_consumers at the same time?
>
> So do we end up with multiple buffers registered and have
> to demux out to the read_raw + the actual buffered path?
> Given we have a bit of code saying grab one sample, I'm
> going to guess we don't...
>
> If so, the vast majority of the buffer setup code in IIO
> is irrelevant here and we just need to call a few of
> the callbacks from this driver directly... (I think
> though I haven't chased through every corner.
>
> I'd rather avoid introducing this nesting for a corner
> case that makes no 'semantic' sense in IIO as it leaves us
> in two separate states at the same time that the driver
> is trying to make mutually exclusive. We can't both
> not be in buffered mode, and in buffered mode.
>
> Thanks and good luck with this nasty corner!
>
> Jonathan
>
Here I consider the following use case:
A single conversion is performed. The dfsdm (filter) is chained with a
front-end, which can be an ADC or a sensor. So we have two IIO devices,
the dfsdm and its front-end handled through the hw consumer interface.
You are right. There is something wrong here, in buffered/non-buffered
mode mixing.
iio_hw_consumer_enable() call is used to enable the front-end device.
But this interface is intended for buffered mode.
So this is not coherent with the expected single conversion mode,
indeed. Another interface is required to manage the front-end device. I
have a poor knowledge of iio framework, but it seems to me that there is
no interface to manage this.
My understanding regarding mlock, is that it is used to protect the
state of the iio device.
I we want to do a conversion from the chained devices, I think we need
to activate the first device
and keep it performing conversion, as long as the second device has done
its conversion.
We need to protect both devices, and we should have to do it in a nested
way.
So, I guess that anyway, nested mutexes would be required in this case.
Best regards
Olivier
>
>> Here two instances of the same lock class are requested
>> on two different objects.
>> The locking validator needs to be informed of the nesting level
>> of each lock to avoid a false positive.
>>
>> This patch introduces a class hierarchy in iio device lock,
>> assuming that hardware consumer is at a lower level than iio device.
>>
>> [ 52.086174]
>> [ 52.086223] ============================================
>> [ 52.091516] WARNING: possible recursive locking detected
>> [ 52.096825] 4.19.49 #162 Not tainted
>> [ 52.100384] --------------------------------------------
>> [ 52.105691] cat/823 is trying to acquire lock:
>> [ 52.110132] 37acb703 (&dev->mlock){+.+.}, at: iio_update_buffers+0x3c/0xd0
>> [ 52.116995]
>> [ 52.116995] but task is already holding lock:
>> [ 52.122821] 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
>> [ 52.130560]
>> [ 52.130560] other info that might help us debug this:
>> [ 52.137083] Possible unsafe locking scenario:
>> [ 52.137083]
>> [ 52.142995] CPU0
>> [ 52.145430] ----
>> [ 52.147864] lock(&dev->mlock);
>> [ 52.151082] lock(&dev->mlock);
>> [ 52.154301]
>> [ 52.154301] * DEADLOCK *
>> [ 52.154301]
>> [ 52.160215] May be due to missing lock nesting notation
>> [ 52.160215]
>> [ 52.167000] 5 locks held by cat/823:
>> [ 52.170563] #0: 96d6554b (&p->lock){+.+.}, at: seq_read+0x34/0x51c
>> [ 52.176824] #1: 3cf6739a (&of->mutex){+.+.}, at: kernfs_seq_start+0x1c/0x8c
>> [ 52.183866] #2: a6090e0a (kn->count#29){.+.+}, at: kernfs_seq_start+0x24/0x8c
>> [ 52.191083] #3: 368bb908 (&dev->mlock){+.+.}, at: iio_device_claim_direct_mode+0x18/0x34
>> [ 52.199257] #4: 77e2bcfe (&dev->info_exist_lock){+.+.}, at: iio_update_buffers+0x30/0xd0
>> [ 52.207431]
>> [ 52.207431] stack backtrace:
>> [ 52.211787] CPU: 0 PID: 823 Comm: cat Not tainted 4.19.49 #162
>> [ 52.217606] Hardware name: STM32 (Device Tree Support)
>> [ 52.222756] [<c0112420>] (unwind_backtrace) from [<c010df5c>] (show_stack+0x10/0x14)
>> [ 52.230487] [<c010df5c>] (show_stack) from [<c0af5c88>] (dump_stack+0xc4/0xf0)
>> [ 52.237703] [<c0af5c88>] (dump_stack) from [<c01865bc>] (__lock_acquire+0x874/0x1344)
>> [ 52.245525] [<c01865bc>] (__lock_acquire) from [<c0187be8>] (lock_acquire+0xd8/0x268)
>> [ 52.253353] [<c0187be8>] (lock_acquire) from [<c0b0dcf8>] (__mutex_lock+0x70/0xab0)
>> [ 52.261005] [<c0b0dcf8>] (__mutex_lock) from [<c0b0e754>] (mutex_lock_nested+0x1c/0x24)
>> [ 52.269001] [<c0b0e754>] (mutex_lock_nested) from [<c09282b8>] (iio_update_buffers+0x3c/0xd0)
>> [ 52.277523] [<c09282b8>] (iio_update_buffers) from [<c09329cc>] (iio_hw_consumer_enable+0x34/0x70)
>> [ 52.286476] [<c09329cc>] (iio_hw_consumer_enable) from [<c0932134>] (stm32_dfsdm_read_raw+0xf4/0x3fc)
>> [ 52.295695] [<c0932134>] (stm32_dfsdm_read_raw) from [<c0922eb4>] (iio_read_channel_info+0xa8/0xb0)
>> [ 52.304738] [<c0922eb4>] (iio_read_channel_info) from [<c067a7fc>] (dev_attr_show+0x1c/0x48)
>> [ 52.313170] [<c067a7fc>] (dev_attr_show) from [<c03724a4>] (sysfs_kf_seq_show+0x84/0xec)
>> [ 52.321256] [<c03724a4>] (sysfs_kf_seq_show) from [<c0312afc>] (seq_read+0x154/0x51c)
>> [ 52.329082] [<c0312afc>] (seq_read) from [<c02e7a00>] (__vfs_read+0x2c/0x15c)
>> [ 52.336209] [<c02e7a00>] (__vfs_read) from [<c02e7bc0>] (vfs_read+0x90/0x15c)
>> [ 52.343339] [<c02e7bc0>] (vfs_read) from [<c02e81ac>] (ksys_read+0x5c/0xdc)
>> [ 52.350296] [<c02e81ac>] (ksys_read) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
>> [ 52.357852] Exception stack(0xe5761fa8 to 0xe5761ff0)
>> [ 52.362904] 1fa0: 0000006c 7ff00000 00000003 b6e06000 00020000 00000000
>> [ 52.371077] 1fc0: 0000006c 7ff00000 00020000 00000003 00000003 00000000 00020000 00000000
>> [ 52.379245] 1fe0: 00000003 beb6e790 b6eb17b7 b6e3e6c6
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
>> ---
>> drivers/iio/buffer/industrialio-hw-consumer.c | 9 ++++++++-
>> drivers/iio/industrialio-buffer.c | 2 +-
>> drivers/iio/industrialio-core.c | 3 ++-
>> include/linux/iio/iio.h | 6 ++++++
>> 4 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
>> index 95165697d8ae..652ce31b4b5f 100644
>> --- a/drivers/iio/buffer/industrialio-hw-consumer.c
>> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
>> @@ -101,6 +101,7 @@ struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
>>
>> chan = &hwc->channels[0];
>> while (chan->indio_dev) {
>> + chan->indio_dev->mutex_class = IIO_MUTEX_HWC;
>> buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
>> if (!buf) {
>> ret = -ENOMEM;
>> @@ -129,8 +130,14 @@ EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
>> void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
>> {
>> struct hw_consumer_buffer *buf, *n;
>> + struct iio_channel *chan = &hwc->channels[0];
>> +
>> + while (chan->indio_dev) {
>> + chan->indio_dev->mutex_class = IIO_MUTEX_NORMAL;
>> + iio_channel_release(chan);
>> + chan++;
>> + }
>>
>> - iio_channel_release_all(hwc->channels);
>> list_for_each_entry_safe(buf, n, &hwc->buffers, head)
>> iio_buffer_put(&buf->buffer);
>> kfree(hwc);
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index c193d64e5217..d1df04167978 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -1077,7 +1077,7 @@ int iio_update_buffers(struct iio_dev *indio_dev,
>> return 0;
>>
>> mutex_lock(&indio_dev->info_exist_lock);
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>>
>> if (insert_buffer && iio_buffer_is_active(insert_buffer))
>> insert_buffer = NULL;
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index f72c2dc5f703..b14ba42559a3 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -1454,6 +1454,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
>> dev->dev.groups = dev->groups;
>> dev->dev.type = &iio_device_type;
>> dev->dev.bus = &iio_bus_type;
>> + dev->mutex_class = IIO_MUTEX_NORMAL;
>> device_initialize(&dev->dev);
>> dev_set_drvdata(&dev->dev, (void *)dev);
>> mutex_init(&dev->mlock);
>> @@ -1805,7 +1806,7 @@ EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
>> */
>> int iio_device_claim_direct_mode(struct iio_dev *indio_dev)
>> {
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock_nested(&indio_dev->mlock, indio_dev->mutex_class);
>>
>> if (iio_buffer_enabled(indio_dev)) {
>> mutex_unlock(&indio_dev->mlock);
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index 862ce0019eba..1192eca124f4 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -17,6 +17,11 @@
>> * Currently assumes nano seconds.
>> */
>>
>> +enum iio_mutex_lock_class {
>> + IIO_MUTEX_NORMAL,
>> + IIO_MUTEX_HWC,
>> +};
>> +
>> enum iio_shared_by {
>> IIO_SEPARATE,
>> IIO_SHARED_BY_TYPE,
>> @@ -537,6 +542,7 @@ struct iio_dev {
>> struct list_head buffer_list;
>> int scan_bytes;
>> struct mutex mlock;
>> + int mutex_class;
>>
>> const unsigned long *available_scan_masks;
>> unsigned masklength;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
2019-10-14 15:59 ` Olivier MOYSAN
@ 2019-10-15 21:11 ` Lars-Peter Clausen
-1 siblings, 0 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2019-10-15 21:11 UTC (permalink / raw)
To: Olivier MOYSAN, Jonathan Cameron
Cc: knaack.h, pmeerw, mcoquelin.stm32, Alexandre TORGUE,
Fabrice GASNIER, linux-iio, linux-stm32, linux-arm-kernel,
linux-kernel, Benjamin GAIGNARD
On 10/14/19 5:59 PM, Olivier MOYSAN wrote:
> Hello Jonathan,
>
> Thanks for your comment.
>
> On 10/12/19 10:57 AM, Jonathan Cameron wrote:
>> On Fri, 11 Oct 2019 17:13:14 +0200
>> Olivier Moysan <olivier.moysan@st.com> wrote:
>>
>>> The aim of this patch is to correct a recursive locking warning,
>>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
>>> This message was initially triggered by the following call sequence
>>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>>>
>>> in stm32_dfsdm_read_raw()
>>> iio_device_claim_direct_mode
>>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
>>> iio_hw_consumer_enable
>>> iio_update_buffers
>>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
>> Hmm. I'm not sure I follow the logic. That lock is
>> for one thing and one thing only, preventing access
>> to the iio device that are unsafe when it is running
>> in a buffered mode. We shouldn't be in a position where
>> we both say don't do this if we are in buffered mode, + enter
>> buffered mode whilst doing this, or we need special functions
>> for entering buffering mode if in this state. We are in
>> some sense combining internal driver logic with overall
>> IIO states. IIO shouldn't care that the device is using
>> the same methods under the hood for buffered and non
>> buffered operations.
>>
>> I can't really recall how this driver works. Is it actually
>> possible to have multiple hw_consumers at the same time?
>>
>> So do we end up with multiple buffers registered and have
>> to demux out to the read_raw + the actual buffered path?
>> Given we have a bit of code saying grab one sample, I'm
>> going to guess we don't...
>>
>> If so, the vast majority of the buffer setup code in IIO
>> is irrelevant here and we just need to call a few of
>> the callbacks from this driver directly... (I think
>> though I haven't chased through every corner.
>>
>> I'd rather avoid introducing this nesting for a corner
>> case that makes no 'semantic' sense in IIO as it leaves us
>> in two separate states at the same time that the driver
>> is trying to make mutually exclusive. We can't both
>> not be in buffered mode, and in buffered mode.
>>
>> Thanks and good luck with this nasty corner!
>>
>> Jonathan
>>
> Here I consider the following use case:
> A single conversion is performed. The dfsdm (filter) is chained with a
> front-end, which can be an ADC or a sensor. So we have two IIO devices,
> the dfsdm and its front-end handled through the hw consumer interface.
>
> You are right. There is something wrong here, in buffered/non-buffered
> mode mixing.
> iio_hw_consumer_enable() call is used to enable the front-end device.
> But this interface is intended for buffered mode.
> So this is not coherent with the expected single conversion mode,
> indeed. Another interface is required to manage the front-end device. I
> have a poor knowledge of iio framework, but it seems to me that there is
> no interface to manage this.
>
> My understanding regarding mlock, is that it is used to protect the
> state of the iio device.
> I we want to do a conversion from the chained devices, I think we need
> to activate the first device
> and keep it performing conversion, as long as the second device has done
> its conversion.
> We need to protect both devices, and we should have to do it in a nested
> way.
> So, I guess that anyway, nested mutexes would be required in this case.
>
Others like regmap have solved this by having a lockclass per instance.
Although that is not ideal either since it will slow down lockdep.
See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/regmap.h#n629
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-15 21:11 ` Lars-Peter Clausen
0 siblings, 0 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2019-10-15 21:11 UTC (permalink / raw)
To: Olivier MOYSAN, Jonathan Cameron
Cc: Alexandre TORGUE, linux-iio, pmeerw, linux-kernel,
mcoquelin.stm32, knaack.h, Fabrice GASNIER, linux-stm32,
linux-arm-kernel, Benjamin GAIGNARD
On 10/14/19 5:59 PM, Olivier MOYSAN wrote:
> Hello Jonathan,
>
> Thanks for your comment.
>
> On 10/12/19 10:57 AM, Jonathan Cameron wrote:
>> On Fri, 11 Oct 2019 17:13:14 +0200
>> Olivier Moysan <olivier.moysan@st.com> wrote:
>>
>>> The aim of this patch is to correct a recursive locking warning,
>>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
>>> This message was initially triggered by the following call sequence
>>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
>>>
>>> in stm32_dfsdm_read_raw()
>>> iio_device_claim_direct_mode
>>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
>>> iio_hw_consumer_enable
>>> iio_update_buffers
>>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
>> Hmm. I'm not sure I follow the logic. That lock is
>> for one thing and one thing only, preventing access
>> to the iio device that are unsafe when it is running
>> in a buffered mode. We shouldn't be in a position where
>> we both say don't do this if we are in buffered mode, + enter
>> buffered mode whilst doing this, or we need special functions
>> for entering buffering mode if in this state. We are in
>> some sense combining internal driver logic with overall
>> IIO states. IIO shouldn't care that the device is using
>> the same methods under the hood for buffered and non
>> buffered operations.
>>
>> I can't really recall how this driver works. Is it actually
>> possible to have multiple hw_consumers at the same time?
>>
>> So do we end up with multiple buffers registered and have
>> to demux out to the read_raw + the actual buffered path?
>> Given we have a bit of code saying grab one sample, I'm
>> going to guess we don't...
>>
>> If so, the vast majority of the buffer setup code in IIO
>> is irrelevant here and we just need to call a few of
>> the callbacks from this driver directly... (I think
>> though I haven't chased through every corner.
>>
>> I'd rather avoid introducing this nesting for a corner
>> case that makes no 'semantic' sense in IIO as it leaves us
>> in two separate states at the same time that the driver
>> is trying to make mutually exclusive. We can't both
>> not be in buffered mode, and in buffered mode.
>>
>> Thanks and good luck with this nasty corner!
>>
>> Jonathan
>>
> Here I consider the following use case:
> A single conversion is performed. The dfsdm (filter) is chained with a
> front-end, which can be an ADC or a sensor. So we have two IIO devices,
> the dfsdm and its front-end handled through the hw consumer interface.
>
> You are right. There is something wrong here, in buffered/non-buffered
> mode mixing.
> iio_hw_consumer_enable() call is used to enable the front-end device.
> But this interface is intended for buffered mode.
> So this is not coherent with the expected single conversion mode,
> indeed. Another interface is required to manage the front-end device. I
> have a poor knowledge of iio framework, but it seems to me that there is
> no interface to manage this.
>
> My understanding regarding mlock, is that it is used to protect the
> state of the iio device.
> I we want to do a conversion from the chained devices, I think we need
> to activate the first device
> and keep it performing conversion, as long as the second device has done
> its conversion.
> We need to protect both devices, and we should have to do it in a nested
> way.
> So, I guess that anyway, nested mutexes would be required in this case.
>
Others like regmap have solved this by having a lockclass per instance.
Although that is not ideal either since it will slow down lockdep.
See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/regmap.h#n629
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
2019-10-15 21:11 ` Lars-Peter Clausen
@ 2019-10-22 11:53 ` Jonathan Cameron
-1 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2019-10-22 11:53 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Olivier MOYSAN, knaack.h, pmeerw, mcoquelin.stm32,
Alexandre TORGUE, Fabrice GASNIER, linux-iio, linux-stm32,
linux-arm-kernel, linux-kernel, Benjamin GAIGNARD
On Tue, 15 Oct 2019 23:11:43 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/14/19 5:59 PM, Olivier MOYSAN wrote:
> > Hello Jonathan,
> >
> > Thanks for your comment.
> >
> > On 10/12/19 10:57 AM, Jonathan Cameron wrote:
> >> On Fri, 11 Oct 2019 17:13:14 +0200
> >> Olivier Moysan <olivier.moysan@st.com> wrote:
> >>
> >>> The aim of this patch is to correct a recursive locking warning,
> >>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
> >>> This message was initially triggered by the following call sequence
> >>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
> >>>
> >>> in stm32_dfsdm_read_raw()
> >>> iio_device_claim_direct_mode
> >>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
> >>> iio_hw_consumer_enable
> >>> iio_update_buffers
> >>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
> >> Hmm. I'm not sure I follow the logic. That lock is
> >> for one thing and one thing only, preventing access
> >> to the iio device that are unsafe when it is running
> >> in a buffered mode. We shouldn't be in a position where
> >> we both say don't do this if we are in buffered mode, + enter
> >> buffered mode whilst doing this, or we need special functions
> >> for entering buffering mode if in this state. We are in
> >> some sense combining internal driver logic with overall
> >> IIO states. IIO shouldn't care that the device is using
> >> the same methods under the hood for buffered and non
> >> buffered operations.
> >>
> >> I can't really recall how this driver works. Is it actually
> >> possible to have multiple hw_consumers at the same time?
> >>
> >> So do we end up with multiple buffers registered and have
> >> to demux out to the read_raw + the actual buffered path?
> >> Given we have a bit of code saying grab one sample, I'm
> >> going to guess we don't...
> >>
> >> If so, the vast majority of the buffer setup code in IIO
> >> is irrelevant here and we just need to call a few of
> >> the callbacks from this driver directly... (I think
> >> though I haven't chased through every corner.
> >>
> >> I'd rather avoid introducing this nesting for a corner
> >> case that makes no 'semantic' sense in IIO as it leaves us
> >> in two separate states at the same time that the driver
> >> is trying to make mutually exclusive. We can't both
> >> not be in buffered mode, and in buffered mode.
> >>
> >> Thanks and good luck with this nasty corner!
> >>
> >> Jonathan
> >>
> > Here I consider the following use case:
> > A single conversion is performed. The dfsdm (filter) is chained with a
> > front-end, which can be an ADC or a sensor. So we have two IIO devices,
> > the dfsdm and its front-end handled through the hw consumer interface.
> >
> > You are right. There is something wrong here, in buffered/non-buffered
> > mode mixing.
> > iio_hw_consumer_enable() call is used to enable the front-end device.
> > But this interface is intended for buffered mode.
> > So this is not coherent with the expected single conversion mode,
> > indeed. Another interface is required to manage the front-end device. I
> > have a poor knowledge of iio framework, but it seems to me that there is
> > no interface to manage this.
> >
> > My understanding regarding mlock, is that it is used to protect the
> > state of the iio device.
> > I we want to do a conversion from the chained devices, I think we need
> > to activate the first device
> > and keep it performing conversion, as long as the second device has done
> > its conversion.
> > We need to protect both devices, and we should have to do it in a nested
> > way.
> > So, I guess that anyway, nested mutexes would be required in this case.
> >
>
> Others like regmap have solved this by having a lockclass per instance.
> Although that is not ideal either since it will slow down lockdep.
>
> See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/regmap.h#n629
It'll take me a while to get back to this as my understanding is
currently very limited. Poke me if I've not replied in a few weeks.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFC] iio: core: add a class hierarchy on iio device lock
@ 2019-10-22 11:53 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2019-10-22 11:53 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Olivier MOYSAN, Alexandre TORGUE, linux-iio, pmeerw,
linux-kernel, mcoquelin.stm32, knaack.h, Fabrice GASNIER,
linux-stm32, linux-arm-kernel, Benjamin GAIGNARD
On Tue, 15 Oct 2019 23:11:43 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/14/19 5:59 PM, Olivier MOYSAN wrote:
> > Hello Jonathan,
> >
> > Thanks for your comment.
> >
> > On 10/12/19 10:57 AM, Jonathan Cameron wrote:
> >> On Fri, 11 Oct 2019 17:13:14 +0200
> >> Olivier Moysan <olivier.moysan@st.com> wrote:
> >>
> >>> The aim of this patch is to correct a recursive locking warning,
> >>> detected when setting CONFIG_PROVE_LOCKING flag (as shown in message below).
> >>> This message was initially triggered by the following call sequence
> >>> in stm32-dfsdm-adc.c driver, when using IIO hardware consumer interface.
> >>>
> >>> in stm32_dfsdm_read_raw()
> >>> iio_device_claim_direct_mode
> >>> mutex_lock(&indio_dev->mlock); -> lock on dfsdm device
> >>> iio_hw_consumer_enable
> >>> iio_update_buffers
> >>> mutex_lock(&indio_dev->mlock); -> lock on hw consumer device
> >> Hmm. I'm not sure I follow the logic. That lock is
> >> for one thing and one thing only, preventing access
> >> to the iio device that are unsafe when it is running
> >> in a buffered mode. We shouldn't be in a position where
> >> we both say don't do this if we are in buffered mode, + enter
> >> buffered mode whilst doing this, or we need special functions
> >> for entering buffering mode if in this state. We are in
> >> some sense combining internal driver logic with overall
> >> IIO states. IIO shouldn't care that the device is using
> >> the same methods under the hood for buffered and non
> >> buffered operations.
> >>
> >> I can't really recall how this driver works. Is it actually
> >> possible to have multiple hw_consumers at the same time?
> >>
> >> So do we end up with multiple buffers registered and have
> >> to demux out to the read_raw + the actual buffered path?
> >> Given we have a bit of code saying grab one sample, I'm
> >> going to guess we don't...
> >>
> >> If so, the vast majority of the buffer setup code in IIO
> >> is irrelevant here and we just need to call a few of
> >> the callbacks from this driver directly... (I think
> >> though I haven't chased through every corner.
> >>
> >> I'd rather avoid introducing this nesting for a corner
> >> case that makes no 'semantic' sense in IIO as it leaves us
> >> in two separate states at the same time that the driver
> >> is trying to make mutually exclusive. We can't both
> >> not be in buffered mode, and in buffered mode.
> >>
> >> Thanks and good luck with this nasty corner!
> >>
> >> Jonathan
> >>
> > Here I consider the following use case:
> > A single conversion is performed. The dfsdm (filter) is chained with a
> > front-end, which can be an ADC or a sensor. So we have two IIO devices,
> > the dfsdm and its front-end handled through the hw consumer interface.
> >
> > You are right. There is something wrong here, in buffered/non-buffered
> > mode mixing.
> > iio_hw_consumer_enable() call is used to enable the front-end device.
> > But this interface is intended for buffered mode.
> > So this is not coherent with the expected single conversion mode,
> > indeed. Another interface is required to manage the front-end device. I
> > have a poor knowledge of iio framework, but it seems to me that there is
> > no interface to manage this.
> >
> > My understanding regarding mlock, is that it is used to protect the
> > state of the iio device.
> > I we want to do a conversion from the chained devices, I think we need
> > to activate the first device
> > and keep it performing conversion, as long as the second device has done
> > its conversion.
> > We need to protect both devices, and we should have to do it in a nested
> > way.
> > So, I guess that anyway, nested mutexes would be required in this case.
> >
>
> Others like regmap have solved this by having a lockclass per instance.
> Although that is not ideal either since it will slow down lockdep.
>
> See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/regmap.h#n629
It'll take me a while to get back to this as my understanding is
currently very limited. Poke me if I've not replied in a few weeks.
Thanks,
Jonathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-22 11:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 15:13 [PATCH][RFC] iio: core: add a class hierarchy on iio device lock Olivier Moysan
2019-10-11 15:13 ` Olivier Moysan
2019-10-12 8:57 ` Jonathan Cameron
2019-10-12 8:57 ` Jonathan Cameron
2019-10-14 15:59 ` Olivier MOYSAN
2019-10-14 15:59 ` Olivier MOYSAN
2019-10-15 21:11 ` Lars-Peter Clausen
2019-10-15 21:11 ` Lars-Peter Clausen
2019-10-22 11:53 ` Jonathan Cameron
2019-10-22 11:53 ` Jonathan Cameron
2019-10-14 4:06 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.