inst->handles is traversed using list_for_each_entry_rcu
outside an RCU read-side critical section but under the protection
of knav_dev_lock.
Hence, add corresponding lockdep expression to silence false-positive
lockdep warnings, and harden RCU lists.
Add macro for the corresponding lockdep expression to make the code
clean and concise.
Signed-off-by: Amol Grover <[email protected]>
---
drivers/soc/ti/knav_qmss_queue.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 1ccc9064e1eb..888dc091c63b 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -25,6 +25,8 @@
static struct knav_device *kdev;
static DEFINE_MUTEX(knav_dev_lock);
+#define knav_dev_lock_held() \
+ lockdep_is_held(&knav_dev_lock)
/* Queue manager register indices in DTS */
#define KNAV_QUEUE_PEEK_REG_INDEX 0
@@ -52,8 +54,9 @@ static DEFINE_MUTEX(knav_dev_lock);
#define knav_queue_idx_to_inst(kdev, idx) \
(kdev->instances + (idx << kdev->inst_shift))
-#define for_each_handle_rcu(qh, inst) \
- list_for_each_entry_rcu(qh, &inst->handles, list)
+#define for_each_handle_rcu(qh, inst) \
+ list_for_each_entry_rcu(qh, &inst->handles, list, \
+ rcu_read_lock_held() || knav_dev_lock_held())
#define for_each_instance(idx, inst, kdev) \
for (idx = 0, inst = kdev->instances; \
--
2.24.1
Hi Amol,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on arm-soc/for-next clk/clk-next linus/master keystone/next v5.5-rc5 next-20200109]
[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/Amol-Grover/drivers-soc-ti-knav_qmss_queue-Pass-lockdep-expression-to-RCU-lists/20200111-054347
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1522d9da40bdfe502c91163e6d769332897201fa
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/rculist.h:11:0,
from include/linux/dcache.h:7,
from include/linux/fs.h:8,
from include/linux/debugfs.h:15,
from drivers/soc/ti/knav_qmss_queue.c:11:
drivers/soc/ti/knav_qmss_queue.c: In function 'knav_queue_notify':
include/linux/rculist.h:53:25: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(), \
include/linux/rcupdate.h:263:52: note: in definition of macro 'RCU_LOCKDEP_WARN'
if (debug_lockdep_rcu_enabled() && !__warned && (c)) { \
^
include/linux/rculist.h:371:7: note: in expansion of macro '__list_check_rcu'
for (__list_check_rcu(dummy, ## cond, 0), \
^~~~~~~~~~~~~~~~
>> drivers/soc/ti/knav_qmss_queue.c:58:2: note: in expansion of macro 'list_for_each_entry_rcu'
list_for_each_entry_rcu(qh, &inst->handles, list, \
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/soc/ti/knav_qmss_queue.c:92:2: note: in expansion of macro 'for_each_handle_rcu'
for_each_handle_rcu(qh, inst) {
^~~~~~~~~~~~~~~~~~~
drivers/soc/ti/knav_qmss_queue.c: In function 'knav_queue_is_shared':
include/linux/rculist.h:53:25: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(), \
include/linux/rcupdate.h:263:52: note: in definition of macro 'RCU_LOCKDEP_WARN'
if (debug_lockdep_rcu_enabled() && !__warned && (c)) { \
^
include/linux/rculist.h:371:7: note: in expansion of macro '__list_check_rcu'
for (__list_check_rcu(dummy, ## cond, 0), \
^~~~~~~~~~~~~~~~
>> drivers/soc/ti/knav_qmss_queue.c:58:2: note: in expansion of macro 'list_for_each_entry_rcu'
list_for_each_entry_rcu(qh, &inst->handles, list, \
^~~~~~~~~~~~~~~~~~~~~~~
drivers/soc/ti/knav_qmss_queue.c:165:2: note: in expansion of macro 'for_each_handle_rcu'
for_each_handle_rcu(tmp, inst) {
^~~~~~~~~~~~~~~~~~~
drivers/soc/ti/knav_qmss_queue.c: In function 'knav_queue_debug_show_instance':
include/linux/rculist.h:53:25: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(), \
include/linux/rcupdate.h:263:52: note: in definition of macro 'RCU_LOCKDEP_WARN'
if (debug_lockdep_rcu_enabled() && !__warned && (c)) { \
^
include/linux/rculist.h:371:7: note: in expansion of macro '__list_check_rcu'
for (__list_check_rcu(dummy, ## cond, 0), \
^~~~~~~~~~~~~~~~
>> drivers/soc/ti/knav_qmss_queue.c:58:2: note: in expansion of macro 'list_for_each_entry_rcu'
list_for_each_entry_rcu(qh, &inst->handles, list, \
^~~~~~~~~~~~~~~~~~~~~~~
drivers/soc/ti/knav_qmss_queue.c:445:2: note: in expansion of macro 'for_each_handle_rcu'
for_each_handle_rcu(qh, inst) {
^~~~~~~~~~~~~~~~~~~
vim +/list_for_each_entry_rcu +58 drivers/soc/ti/knav_qmss_queue.c
53
54 #define knav_queue_idx_to_inst(kdev, idx) \
55 (kdev->instances + (idx << kdev->inst_shift))
56
57 #define for_each_handle_rcu(qh, inst) \
> 58 list_for_each_entry_rcu(qh, &inst->handles, list, \
59 rcu_read_lock_held() || knav_dev_lock_held())
60
61 #define for_each_instance(idx, inst, kdev) \
62 for (idx = 0, inst = kdev->instances; \
63 idx < (kdev)->num_queues_in_use; \
64 idx++, inst = knav_queue_idx_to_inst(kdev, idx))
65
66 /* All firmware file names end up here. List the firmware file names below.
67 * Newest followed by older ones. Search is done from start of the array
68 * until a firmware file is found.
69 */
70 const char *knav_acc_firmwares[] = {"ks2_qmss_pdsp_acc48.bin"};
71
72 static bool device_ready;
73 bool knav_qmss_device_ready(void)
74 {
75 return device_ready;
76 }
77 EXPORT_SYMBOL_GPL(knav_qmss_device_ready);
78
79 /**
80 * knav_queue_notify: qmss queue notfier call
81 *
82 * @inst: qmss queue instance like accumulator
83 */
84 void knav_queue_notify(struct knav_queue_inst *inst)
85 {
86 struct knav_queue *qh;
87
88 if (!inst)
89 return;
90
91 rcu_read_lock();
> 92 for_each_handle_rcu(qh, inst) {
93 if (atomic_read(&qh->notifier_enabled) <= 0)
94 continue;
95 if (WARN_ON(!qh->notifier_fn))
96 continue;
97 this_cpu_inc(qh->stats->notifies);
98 qh->notifier_fn(qh->notifier_fn_arg);
99 }
100 rcu_read_unlock();
101 }
102 EXPORT_SYMBOL_GPL(knav_queue_notify);
103
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation