* [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
2019-06-03 8:01 ` Peter Zijlstra
2019-06-04 14:01 ` Rasmus Villemoes
2019-06-01 22:27 ` [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry Joel Fernandes (Google)
` (4 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
This patch adds support for checking RCU reader sections in list
traversal macros. Optionally, if the list macro is called under SRCU or
other lock/mutex protection, then appropriate lockdep expressions can be
passed to make the checks pass.
Existing list_for_each_entry_rcu() invocations don't need to pass the
optional fourth argument (cond) unless they are under some non-RCU
protection and needs to make lockdep check pass.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
include/linux/rculist.h | 40 ++++++++++++++++++++++++++++++++++++----
include/linux/rcupdate.h | 7 +++++++
kernel/rcu/update.c | 26 ++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index e91ec9ddcd30..b641fdd9f1a2 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -40,6 +40,25 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
*/
#define list_next_rcu(list) (*((struct list_head __rcu **)(&(list)->next)))
+/*
+ * Check during list traversal that we are within an RCU reader
+ */
+#define __list_check_rcu() \
+ RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), \
+ "RCU-list traversed in non-reader section!")
+
+static inline void __list_check_rcu_cond(int dummy, ...)
+{
+ va_list ap;
+ int cond;
+
+ va_start(ap, dummy);
+ cond = va_arg(ap, int);
+ va_end(ap);
+
+ RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(),
+ "RCU-list traversed in non-reader section!");
+}
/*
* Insert a new entry between two known consecutive entries.
*
@@ -338,6 +357,9 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
member) : NULL; \
})
+#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
+#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
+
/**
* list_for_each_entry_rcu - iterate over rcu list of given type
* @pos: the type * to use as a loop cursor.
@@ -348,9 +370,14 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
* the _rcu list-mutation primitives such as list_add_rcu()
* as long as the traversal is guarded by rcu_read_lock().
*/
-#define list_for_each_entry_rcu(pos, head, member) \
- for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
- &pos->member != (head); \
+#define list_for_each_entry_rcu(pos, head, member, cond...) \
+ if (COUNT_VARGS(cond) != 0) { \
+ __list_check_rcu_cond(0, ## cond); \
+ } else { \
+ __list_check_rcu(); \
+ } \
+ for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
+ &pos->member != (head); \
pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
/**
@@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* the _rcu list-mutation primitives such as hlist_add_head_rcu()
* as long as the traversal is guarded by rcu_read_lock().
*/
-#define hlist_for_each_entry_rcu(pos, head, member) \
+#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
+ if (COUNT_VARGS(cond) != 0) { \
+ __list_check_rcu_cond(0, ## cond); \
+ } else { \
+ __list_check_rcu(); \
+ } \
for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 922bb6848813..712b464ab960 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -223,6 +223,7 @@ int debug_lockdep_rcu_enabled(void);
int rcu_read_lock_held(void);
int rcu_read_lock_bh_held(void);
int rcu_read_lock_sched_held(void);
+int rcu_read_lock_any_held(void);
#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -243,6 +244,12 @@ static inline int rcu_read_lock_sched_held(void)
{
return !preemptible();
}
+
+static inline int rcu_read_lock_any_held(void)
+{
+ return !preemptible();
+}
+
#endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
#ifdef CONFIG_PROVE_RCU
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index c3bf44ba42e5..9cb30006a5e1 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -298,6 +298,32 @@ int rcu_read_lock_bh_held(void)
}
EXPORT_SYMBOL_GPL(rcu_read_lock_bh_held);
+int rcu_read_lock_any_held(void)
+{
+ int lockdep_opinion = 0;
+
+ if (!debug_lockdep_rcu_enabled())
+ return 1;
+ if (!rcu_is_watching())
+ return 0;
+ if (!rcu_lockdep_current_cpu_online())
+ return 0;
+
+ /* Preemptible RCU flavor */
+ if (lock_is_held(&rcu_lock_map))
+ return 1;
+
+ /* BH flavor */
+ if (in_softirq() || irqs_disabled())
+ return 1;
+
+ /* Sched flavor */
+ if (debug_locks)
+ lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
+ return lockdep_opinion || !preemptible();
+}
+EXPORT_SYMBOL_GPL(rcu_read_lock_any_held);
+
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
/**
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-01 22:27 ` [RFC 1/6] rcu: Add support for consolidated-RCU reader checking Joel Fernandes (Google)
@ 2019-06-03 8:01 ` Peter Zijlstra
2019-06-03 14:18 ` Joel Fernandes
2019-06-04 14:01 ` Rasmus Villemoes
1 sibling, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2019-06-03 8:01 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Sat, Jun 01, 2019 at 06:27:33PM -0400, Joel Fernandes (Google) wrote:
> +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> + if (COUNT_VARGS(cond) != 0) { \
> + __list_check_rcu_cond(0, ## cond); \
> + } else { \
> + __list_check_rcu(); \
> + } \
> + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> + &pos->member != (head); \
> pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
>
> /**
> @@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> * as long as the traversal is guarded by rcu_read_lock().
> */
> +#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> + if (COUNT_VARGS(cond) != 0) { \
> + __list_check_rcu_cond(0, ## cond); \
> + } else { \
> + __list_check_rcu(); \
> + } \
> for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
This breaks code like:
if (...)
list_for_each_entry_rcu(...);
as they are no longer a single statement. You'll have to frob it into
the initializer part of the for statement.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-03 8:01 ` Peter Zijlstra
@ 2019-06-03 14:18 ` Joel Fernandes
2019-06-03 19:42 ` Joel Fernandes
2019-06-04 10:53 ` Steven Rostedt
0 siblings, 2 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-03 14:18 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Mon, Jun 03, 2019 at 10:01:28AM +0200, Peter Zijlstra wrote:
> On Sat, Jun 01, 2019 at 06:27:33PM -0400, Joel Fernandes (Google) wrote:
> > +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> > + if (COUNT_VARGS(cond) != 0) { \
> > + __list_check_rcu_cond(0, ## cond); \
> > + } else { \
> > + __list_check_rcu(); \
> > + } \
> > + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > + &pos->member != (head); \
> > pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
> >
> > /**
> > @@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> > * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> > * as long as the traversal is guarded by rcu_read_lock().
> > */
> > +#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> > + if (COUNT_VARGS(cond) != 0) { \
> > + __list_check_rcu_cond(0, ## cond); \
> > + } else { \
> > + __list_check_rcu(); \
> > + } \
> > for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> > typeof(*(pos)), member); \
> > pos; \
>
>
> This breaks code like:
>
> if (...)
> list_for_each_entry_rcu(...);
>
> as they are no longer a single statement. You'll have to frob it into
> the initializer part of the for statement.
Thanks a lot for that. I fixed it as below (diff is on top of the patch):
If not for that '##' , I could have abstracted the whole if/else
expression into its own macro and called it from list_for_each_entry_rcu() to
keep it more clean.
---8<-----------------------
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index b641fdd9f1a2..cc742d294bb0 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -371,12 +372,15 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_entry_rcu(pos, head, member, cond...) \
- if (COUNT_VARGS(cond) != 0) { \
- __list_check_rcu_cond(0, ## cond); \
- } else { \
- __list_check_rcu(); \
- } \
- for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
+ for ( \
+ ({ \
+ if (COUNT_VARGS(cond) != 0) { \
+ __list_check_rcu_cond(0, ## cond); \
+ } else { \
+ __list_check_rcu_nocond(); \
+ } \
+ }), \
+ pos = list_entry_rcu((head)->next, typeof(*pos), member); \
&pos->member != (head); \
pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
@@ -649,12 +653,15 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
- if (COUNT_VARGS(cond) != 0) { \
- __list_check_rcu_cond(0, ## cond); \
- } else { \
- __list_check_rcu(); \
- } \
- for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
+ for ( \
+ ({ \
+ if (COUNT_VARGS(cond) != 0) { \
+ __list_check_rcu_cond(0, ## cond); \
+ } else { \
+ __list_check_rcu_nocond(); \
+ } \
+ }), \
+ pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-03 14:18 ` Joel Fernandes
@ 2019-06-03 19:42 ` Joel Fernandes
2019-06-04 10:53 ` Steven Rostedt
1 sibling, 0 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-03 19:42 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Mon, Jun 03, 2019 at 10:18:47AM -0400, Joel Fernandes wrote:
> On Mon, Jun 03, 2019 at 10:01:28AM +0200, Peter Zijlstra wrote:
> > On Sat, Jun 01, 2019 at 06:27:33PM -0400, Joel Fernandes (Google) wrote:
> > > +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> > > + if (COUNT_VARGS(cond) != 0) { \
> > > + __list_check_rcu_cond(0, ## cond); \
> > > + } else { \
> > > + __list_check_rcu(); \
> > > + } \
> > > + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > > + &pos->member != (head); \
> > > pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
> > >
> > > /**
> > > @@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> > > * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> > > * as long as the traversal is guarded by rcu_read_lock().
> > > */
> > > +#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> > > + if (COUNT_VARGS(cond) != 0) { \
> > > + __list_check_rcu_cond(0, ## cond); \
> > > + } else { \
> > > + __list_check_rcu(); \
> > > + } \
> > > for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> > > typeof(*(pos)), member); \
> > > pos; \
> >
> >
> > This breaks code like:
> >
> > if (...)
> > list_for_each_entry_rcu(...);
> >
> > as they are no longer a single statement. You'll have to frob it into
> > the initializer part of the for statement.
>
> Thanks a lot for that. I fixed it as below (diff is on top of the patch):
>
> If not for that '##' , I could have abstracted the whole if/else
> expression into its own macro and called it from list_for_each_entry_rcu() to
> keep it more clean.
Actually was able to roll the if/else into its own macro as well, thus
keeping it clean. thanks!
---8<-----------------------
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index b641fdd9f1a2..cc9c382b080c 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -43,7 +43,11 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
/*
* Check during list traversal that we are within an RCU reader
*/
-#define __list_check_rcu() \
+
+#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
+#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
+
+#define __list_check_rcu_nocond() \
RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), \
"RCU-list traversed in non-reader section!")
@@ -59,6 +63,16 @@ static inline void __list_check_rcu_cond(int dummy, ...)
RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(),
"RCU-list traversed in non-reader section!");
}
+
+#define __list_check_rcu(cond...) \
+ ({ \
+ if (COUNT_VARGS(cond) != 0) { \
+ __list_check_rcu_cond(0, ## cond); \
+ } else { \
+ __list_check_rcu_nocond(); \
+ } \
+ })
+
/*
* Insert a new entry between two known consecutive entries.
*
@@ -357,9 +371,6 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
member) : NULL; \
})
-#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
-#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
-
/**
* list_for_each_entry_rcu - iterate over rcu list of given type
* @pos: the type * to use as a loop cursor.
@@ -371,12 +382,8 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_entry_rcu(pos, head, member, cond...) \
- if (COUNT_VARGS(cond) != 0) { \
- __list_check_rcu_cond(0, ## cond); \
- } else { \
- __list_check_rcu(); \
- } \
- for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
+ for (__list_check_rcu(cond), \
+ pos = list_entry_rcu((head)->next, typeof(*pos), member); \
&pos->member != (head); \
pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
@@ -649,12 +656,8 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
- if (COUNT_VARGS(cond) != 0) { \
- __list_check_rcu_cond(0, ## cond); \
- } else { \
- __list_check_rcu(); \
- } \
- for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
+ for (__list_check_rcu(cond), \
+ pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-03 14:18 ` Joel Fernandes
2019-06-03 19:42 ` Joel Fernandes
@ 2019-06-04 10:53 ` Steven Rostedt
2019-06-04 17:48 ` Joel Fernandes
1 sibling, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2019-06-04 10:53 UTC (permalink / raw)
To: Joel Fernandes
Cc: Peter Zijlstra, linux-kernel, Alexey Kuznetsov, Bjorn Helgaas,
Borislav Petkov, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki, rcu,
Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Mon, 3 Jun 2019 10:18:47 -0400
Joel Fernandes <joel@joelfernandes.org> wrote:
> On Mon, Jun 03, 2019 at 10:01:28AM +0200, Peter Zijlstra wrote:
> > On Sat, Jun 01, 2019 at 06:27:33PM -0400, Joel Fernandes (Google) wrote:
> > > +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> > > + if (COUNT_VARGS(cond) != 0) { \
> > > + __list_check_rcu_cond(0, ## cond); \
> > > + } else { \
> > > + __list_check_rcu(); \
> > > + } \
> > > + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > > + &pos->member != (head); \
> > > pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
> > >
> > > /**
> > > @@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> > > * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> > > * as long as the traversal is guarded by rcu_read_lock().
> > > */
> > > +#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> > > + if (COUNT_VARGS(cond) != 0) { \
> > > + __list_check_rcu_cond(0, ## cond); \
> > > + } else { \
> > > + __list_check_rcu(); \
> > > + } \
> > > for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> > > typeof(*(pos)), member); \
> > > pos; \
> >
> >
> > This breaks code like:
> >
> > if (...)
> > list_for_each_entry_rcu(...);
> >
> > as they are no longer a single statement. You'll have to frob it into
> > the initializer part of the for statement.
>
> Thanks a lot for that. I fixed it as below (diff is on top of the patch):
>
> If not for that '##' , I could have abstracted the whole if/else
> expression into its own macro and called it from list_for_each_entry_rcu() to
> keep it more clean.
>
> ---8<-----------------------
>
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index b641fdd9f1a2..cc742d294bb0 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -371,12 +372,15 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> * as long as the traversal is guarded by rcu_read_lock().
> */
> #define list_for_each_entry_rcu(pos, head, member, cond...) \
> - if (COUNT_VARGS(cond) != 0) { \
> - __list_check_rcu_cond(0, ## cond); \
> - } else { \
> - __list_check_rcu(); \
> - } \
> - for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> + for ( \
> + ({ \
> + if (COUNT_VARGS(cond) != 0) { \
> + __list_check_rcu_cond(0, ## cond); \
> + } else { \
> + __list_check_rcu_nocond(); \
> + } \
> + }), \
For easier to read I would do something like this:
#define check_rcu_list(cond) \
({ \
if (COUNT_VARGS(cond) != 0) \
__list_check_rcu_cond(0, ## cond); \
else \
__list_check_rcu_nocond(); \
})
#define list_for_each_entry_rcu(pos, head, member, cond...) \
for (check_rcu_list(cond), \
-- Steve
> + pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> &pos->member != (head); \
> pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
>
> @@ -649,12 +653,15 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * as long as the traversal is guarded by rcu_read_lock().
> */
> #define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> - if (COUNT_VARGS(cond) != 0) { \
> - __list_check_rcu_cond(0, ## cond); \
> - } else { \
> - __list_check_rcu(); \
> - } \
> - for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> + for ( \
> + ({ \
> + if (COUNT_VARGS(cond) != 0) { \
> + __list_check_rcu_cond(0, ## cond); \
> + } else { \
> + __list_check_rcu_nocond(); \
> + } \
> + }), \
> + pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
> pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-04 10:53 ` Steven Rostedt
@ 2019-06-04 17:48 ` Joel Fernandes
0 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-04 17:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: Peter Zijlstra, linux-kernel, Alexey Kuznetsov, Bjorn Helgaas,
Borislav Petkov, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki, rcu,
Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Tue, Jun 04, 2019 at 06:53:58AM -0400, Steven Rostedt wrote:
> On Mon, 3 Jun 2019 10:18:47 -0400
> Joel Fernandes <joel@joelfernandes.org> wrote:
>
> > On Mon, Jun 03, 2019 at 10:01:28AM +0200, Peter Zijlstra wrote:
> > > On Sat, Jun 01, 2019 at 06:27:33PM -0400, Joel Fernandes (Google) wrote:
> > > > +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> > > > + if (COUNT_VARGS(cond) != 0) { \
> > > > + __list_check_rcu_cond(0, ## cond); \
> > > > + } else { \
> > > > + __list_check_rcu(); \
> > > > + } \
> > > > + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > > > + &pos->member != (head); \
> > > > pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
> > > >
> > > > /**
> > > > @@ -621,7 +648,12 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> > > > * the _rcu list-mutation primitives such as hlist_add_head_rcu()
> > > > * as long as the traversal is guarded by rcu_read_lock().
> > > > */
> > > > +#define hlist_for_each_entry_rcu(pos, head, member, cond...) \
> > > > + if (COUNT_VARGS(cond) != 0) { \
> > > > + __list_check_rcu_cond(0, ## cond); \
> > > > + } else { \
> > > > + __list_check_rcu(); \
> > > > + } \
> > > > for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> > > > typeof(*(pos)), member); \
> > > > pos; \
> > >
> > >
> > > This breaks code like:
> > >
> > > if (...)
> > > list_for_each_entry_rcu(...);
> > >
> > > as they are no longer a single statement. You'll have to frob it into
> > > the initializer part of the for statement.
> >
> > Thanks a lot for that. I fixed it as below (diff is on top of the patch):
> >
> > If not for that '##' , I could have abstracted the whole if/else
> > expression into its own macro and called it from list_for_each_entry_rcu() to
> > keep it more clean.
> >
> > ---8<-----------------------
> >
> > diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> > index b641fdd9f1a2..cc742d294bb0 100644
> > --- a/include/linux/rculist.h
> > +++ b/include/linux/rculist.h
> > @@ -371,12 +372,15 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> > * as long as the traversal is guarded by rcu_read_lock().
> > */
> > #define list_for_each_entry_rcu(pos, head, member, cond...) \
> > - if (COUNT_VARGS(cond) != 0) { \
> > - __list_check_rcu_cond(0, ## cond); \
> > - } else { \
> > - __list_check_rcu(); \
> > - } \
> > - for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > + for ( \
> > + ({ \
> > + if (COUNT_VARGS(cond) != 0) { \
> > + __list_check_rcu_cond(0, ## cond); \
> > + } else { \
> > + __list_check_rcu_nocond(); \
> > + } \
> > + }), \
>
> For easier to read I would do something like this:
>
> #define check_rcu_list(cond) \
> ({ \
> if (COUNT_VARGS(cond) != 0) \
> __list_check_rcu_cond(0, ## cond); \
> else \
> __list_check_rcu_nocond(); \
> })
>
> #define list_for_each_entry_rcu(pos, head, member, cond...) \
> for (check_rcu_list(cond), \
Yes, already doing it this way as I replied to Peter here:
https://lore.kernel.org/patchwork/patch/1082846/#1278489
Thanks!
- Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-01 22:27 ` [RFC 1/6] rcu: Add support for consolidated-RCU reader checking Joel Fernandes (Google)
2019-06-03 8:01 ` Peter Zijlstra
@ 2019-06-04 14:01 ` Rasmus Villemoes
2019-06-04 23:57 ` Joel Fernandes
1 sibling, 1 reply; 21+ messages in thread
From: Rasmus Villemoes @ 2019-06-04 14:01 UTC (permalink / raw)
To: Joel Fernandes (Google), linux-kernel
Cc: Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On 02/06/2019 00.27, Joel Fernandes (Google) wrote:
> This patch adds support for checking RCU reader sections in list
> traversal macros. Optionally, if the list macro is called under SRCU or
> other lock/mutex protection, then appropriate lockdep expressions can be
> passed to make the checks pass.
>
> Existing list_for_each_entry_rcu() invocations don't need to pass the
> optional fourth argument (cond) unless they are under some non-RCU
> protection and needs to make lockdep check pass.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> include/linux/rculist.h | 40 ++++++++++++++++++++++++++++++++++++----
> include/linux/rcupdate.h | 7 +++++++
> kernel/rcu/update.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index e91ec9ddcd30..b641fdd9f1a2 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -40,6 +40,25 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
> */
> #define list_next_rcu(list) (*((struct list_head __rcu **)(&(list)->next)))
>
> +/*
> + * Check during list traversal that we are within an RCU reader
> + */
> +#define __list_check_rcu() \
> + RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), \
> + "RCU-list traversed in non-reader section!")
> +
> +static inline void __list_check_rcu_cond(int dummy, ...)
> +{
> + va_list ap;
> + int cond;
> +
> + va_start(ap, dummy);
> + cond = va_arg(ap, int);
> + va_end(ap);
> +
> + RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(),
> + "RCU-list traversed in non-reader section!");
> +}
> /*
> * Insert a new entry between two known consecutive entries.
> *
> @@ -338,6 +357,9 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> member) : NULL; \
> })
>
> +#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
> +#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
> +> /**
> * list_for_each_entry_rcu - iterate over rcu list of given type
> * @pos: the type * to use as a loop cursor.
> @@ -348,9 +370,14 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> * the _rcu list-mutation primitives such as list_add_rcu()
> * as long as the traversal is guarded by rcu_read_lock().
> */
> -#define list_for_each_entry_rcu(pos, head, member) \
> - for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> - &pos->member != (head); \
> +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> + if (COUNT_VARGS(cond) != 0) { \
> + __list_check_rcu_cond(0, ## cond); \
> + } else { \
> + __list_check_rcu(); \
> + } \
> + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> + &pos->member != (head); \
> pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
Wouldn't something as simple as
#define __list_check_rcu(dummy, cond, ...) \
RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(), \
"RCU-list traversed in non-reader section!");
for ( ({ __list_check_rcu(junk, ##cond, 0); }), pos = ... )
work just as well (i.e., no need for two list_check_rcu and
list_check_rcu_cond variants)? If there's an optional cond, we use that,
if not, we pick the trailing 0, so !cond disappears and it reduces to
your __list_check_rcu(). Moreover, this ensures the RCU_LOCKDEP_WARN
expansion actually picks up the __LINE__ and __FILE__ where the for loop
is used, and not the __FILE__ and __LINE__ of the static inline function
from the header file. It also makes it a bit more type safe/type generic
(if the cond expression happened to have type long or u64 something
rather odd could happen with the inline vararg function).
Rasmus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/6] rcu: Add support for consolidated-RCU reader checking
2019-06-04 14:01 ` Rasmus Villemoes
@ 2019-06-04 23:57 ` Joel Fernandes
0 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-04 23:57 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Tue, Jun 04, 2019 at 04:01:00PM +0200, Rasmus Villemoes wrote:
> On 02/06/2019 00.27, Joel Fernandes (Google) wrote:
> > This patch adds support for checking RCU reader sections in list
> > traversal macros. Optionally, if the list macro is called under SRCU or
> > other lock/mutex protection, then appropriate lockdep expressions can be
> > passed to make the checks pass.
> >
> > Existing list_for_each_entry_rcu() invocations don't need to pass the
> > optional fourth argument (cond) unless they are under some non-RCU
> > protection and needs to make lockdep check pass.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > include/linux/rculist.h | 40 ++++++++++++++++++++++++++++++++++++----
> > include/linux/rcupdate.h | 7 +++++++
> > kernel/rcu/update.c | 26 ++++++++++++++++++++++++++
> > 3 files changed, 69 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> > index e91ec9ddcd30..b641fdd9f1a2 100644
> > --- a/include/linux/rculist.h
> > +++ b/include/linux/rculist.h
> > @@ -40,6 +40,25 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
> > */
> > #define list_next_rcu(list) (*((struct list_head __rcu **)(&(list)->next)))
> >
> > +/*
> > + * Check during list traversal that we are within an RCU reader
> > + */
> > +#define __list_check_rcu() \
> > + RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), \
> > + "RCU-list traversed in non-reader section!")
> > +
> > +static inline void __list_check_rcu_cond(int dummy, ...)
> > +{
> > + va_list ap;
> > + int cond;
> > +
> > + va_start(ap, dummy);
> > + cond = va_arg(ap, int);
> > + va_end(ap);
> > +
> > + RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(),
> > + "RCU-list traversed in non-reader section!");
> > +}
> > /*
> > * Insert a new entry between two known consecutive entries.
> > *
> > @@ -338,6 +357,9 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> > member) : NULL; \
> > })
> >
> > +#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
> > +#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
> > +> /**
> > * list_for_each_entry_rcu - iterate over rcu list of given type
> > * @pos: the type * to use as a loop cursor.
> > @@ -348,9 +370,14 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
> > * the _rcu list-mutation primitives such as list_add_rcu()
> > * as long as the traversal is guarded by rcu_read_lock().
> > */
> > -#define list_for_each_entry_rcu(pos, head, member) \
> > - for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > - &pos->member != (head); \
> > +#define list_for_each_entry_rcu(pos, head, member, cond...) \
> > + if (COUNT_VARGS(cond) != 0) { \
> > + __list_check_rcu_cond(0, ## cond); \
> > + } else { \
> > + __list_check_rcu(); \
> > + } \
> > + for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> > + &pos->member != (head); \
> > pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
>
> Wouldn't something as simple as
>
> #define __list_check_rcu(dummy, cond, ...) \
> RCU_LOCKDEP_WARN(!cond && !rcu_read_lock_any_held(), \
> "RCU-list traversed in non-reader section!");
>
> for ( ({ __list_check_rcu(junk, ##cond, 0); }), pos = ... )
>
> work just as well (i.e., no need for two list_check_rcu and
> list_check_rcu_cond variants)? If there's an optional cond, we use that,
> if not, we pick the trailing 0, so !cond disappears and it reduces to
> your __list_check_rcu(). Moreover, this ensures the RCU_LOCKDEP_WARN
> expansion actually picks up the __LINE__ and __FILE__ where the for loop
> is used, and not the __FILE__ and __LINE__ of the static inline function
> from the header file. It also makes it a bit more type safe/type generic
> (if the cond expression happened to have type long or u64 something
> rather odd could happen with the inline vararg function).
This is much better. I will do it this way. Thank you!
- Joel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 1/6] rcu: Add support for consolidated-RCU reader checking Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
2019-06-02 7:00 ` Pavel Machek
2019-06-01 22:27 ` [RFC 3/6] driver/core: Convert to use built-in RCU list checking Joel Fernandes (Google)
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
net/ipv4/fib_frontend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b298255f6fdb..ef7c9f8e8682 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -127,7 +127,8 @@ struct fib_table *fib_get_table(struct net *net, u32 id)
h = id & (FIB_TABLE_HASHSZ - 1);
head = &net->ipv4.fib_table_hash[h];
- hlist_for_each_entry_rcu(tb, head, tb_hlist) {
+ hlist_for_each_entry_rcu(tb, head, tb_hlist,
+ lockdep_rtnl_is_held()) {
if (tb->tb_id == id)
return tb;
}
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-01 22:27 ` [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry Joel Fernandes (Google)
@ 2019-06-02 7:00 ` Pavel Machek
2019-06-02 12:20 ` Joel Fernandes
0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2019-06-02 7:00 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, peterz, Rafael J. Wysocki, rcu, Steven Rostedt,
Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
This really needs to be merged to previous patch, you can't break
compilation in middle of series...
Or probably you need hlist_for_each_entry_rcu_lockdep() macro with
additional argument, and switch users to it.
Pavel
> net/ipv4/fib_frontend.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index b298255f6fdb..ef7c9f8e8682 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -127,7 +127,8 @@ struct fib_table *fib_get_table(struct net *net, u32 id)
> h = id & (FIB_TABLE_HASHSZ - 1);
>
> head = &net->ipv4.fib_table_hash[h];
> - hlist_for_each_entry_rcu(tb, head, tb_hlist) {
> + hlist_for_each_entry_rcu(tb, head, tb_hlist,
> + lockdep_rtnl_is_held()) {
> if (tb->tb_id == id)
> return tb;
> }
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-02 7:00 ` Pavel Machek
@ 2019-06-02 12:20 ` Joel Fernandes
2019-06-02 12:24 ` Joel Fernandes
0 siblings, 1 reply; 21+ messages in thread
From: Joel Fernandes @ 2019-06-02 12:20 UTC (permalink / raw)
To: Pavel Machek
Cc: LKML, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, Eric Dumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
Kees Cook, Kernel Hardening, Lai Jiangshan, Len Brown,
linux-acpi, linux-pci, Linux PM, Mathieu Desnoyers, Neil Brown,
netdev, Oleg Nesterov, Paul E. McKenney, Peter Zilstra,
Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo,
Thomas Gleixner, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote:
>
> On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> This really needs to be merged to previous patch, you can't break
> compilation in middle of series...
>
> Or probably you need hlist_for_each_entry_rcu_lockdep() macro with
> additional argument, and switch users to it.
Good point. I can also just add a temporary transition macro, and then
remove it in the last patch. That way no new macro is needed.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-02 12:20 ` Joel Fernandes
@ 2019-06-02 12:24 ` Joel Fernandes
2019-06-03 6:42 ` Pavel Machek
0 siblings, 1 reply; 21+ messages in thread
From: Joel Fernandes @ 2019-06-02 12:24 UTC (permalink / raw)
To: Pavel Machek
Cc: LKML, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, Eric Dumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
Kees Cook, Kernel Hardening, Lai Jiangshan, Len Brown,
linux-acpi, linux-pci, Linux PM, Mathieu Desnoyers, Neil Brown,
netdev, Oleg Nesterov, Paul E. McKenney, Peter Zilstra,
Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo,
Thomas Gleixner, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote:
> >
> > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >
> > This really needs to be merged to previous patch, you can't break
> > compilation in middle of series...
> >
> > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with
> > additional argument, and switch users to it.
>
> Good point. I can also just add a temporary transition macro, and then
> remove it in the last patch. That way no new macro is needed.
Actually, no. There is no compilation break so I did not follow what
you mean. The fourth argument to the hlist_for_each_entry_rcu is
optional. The only thing that happens is new lockdep warnings will
arise which later parts of the series fix by passing in that fourth
argument.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-02 12:24 ` Joel Fernandes
@ 2019-06-03 6:42 ` Pavel Machek
2019-06-03 12:28 ` Joel Fernandes
0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2019-06-03 6:42 UTC (permalink / raw)
To: Joel Fernandes
Cc: Pavel Machek, LKML, Alexey Kuznetsov, Bjorn Helgaas,
Borislav Petkov, David S. Miller, Eric Dumazet,
Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
Ingo Molnar, Josh Triplett, Kees Cook, Kernel Hardening,
Lai Jiangshan, Len Brown, linux-acpi, linux-pci, Linux PM,
Mathieu Desnoyers, Neil Brown, netdev, Oleg Nesterov,
Paul E. McKenney, Peter Zilstra, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]
On Sun 2019-06-02 08:24:35, Joel Fernandes wrote:
> On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote:
> > >
> > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > >
> > > This really needs to be merged to previous patch, you can't break
> > > compilation in middle of series...
> > >
> > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with
> > > additional argument, and switch users to it.
> >
> > Good point. I can also just add a temporary transition macro, and then
> > remove it in the last patch. That way no new macro is needed.
>
> Actually, no. There is no compilation break so I did not follow what
> you mean. The fourth argument to the hlist_for_each_entry_rcu is
> optional. The only thing that happens is new lockdep warnings will
> arise which later parts of the series fix by passing in that fourth
> argument.
Sorry, I missed that subtlety. Might be worth it enabling the lockdep
warning last in the series...
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry
2019-06-03 6:42 ` Pavel Machek
@ 2019-06-03 12:28 ` Joel Fernandes
0 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-03 12:28 UTC (permalink / raw)
To: Pavel Machek
Cc: LKML, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, Eric Dumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
Kees Cook, Kernel Hardening, Lai Jiangshan, Len Brown,
linux-acpi, linux-pci, Linux PM, Mathieu Desnoyers, Neil Brown,
netdev, Oleg Nesterov, Paul E. McKenney, Peter Zilstra,
Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo,
Thomas Gleixner, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Mon, Jun 3, 2019 at 2:42 AM Pavel Machek <pavel@denx.de> wrote:
>
> On Sun 2019-06-02 08:24:35, Joel Fernandes wrote:
> > On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> > >
> > > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote:
> > > >
> > > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > >
> > > > This really needs to be merged to previous patch, you can't break
> > > > compilation in middle of series...
> > > >
> > > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with
> > > > additional argument, and switch users to it.
> > >
> > > Good point. I can also just add a temporary transition macro, and then
> > > remove it in the last patch. That way no new macro is needed.
> >
> > Actually, no. There is no compilation break so I did not follow what
> > you mean. The fourth argument to the hlist_for_each_entry_rcu is
> > optional. The only thing that happens is new lockdep warnings will
> > arise which later parts of the series fix by passing in that fourth
> > argument.
>
> Sorry, I missed that subtlety. Might be worth it enabling the lockdep
> warning last in the series...
Good idea, will do! Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 3/6] driver/core: Convert to use built-in RCU list checking
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 1/6] rcu: Add support for consolidated-RCU reader checking Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check Joel Fernandes (Google)
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
list_for_each_entry_rcu has built-in RCU and lock checking. Make use of
it in driver core.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
drivers/base/base.h | 1 +
drivers/base/core.c | 10 ++++++++++
drivers/base/power/runtime.c | 15 ++++++++++-----
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index b405436ee28e..0d32544b6f91 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -165,6 +165,7 @@ static inline int devtmpfs_init(void) { return 0; }
/* Device links support */
extern int device_links_read_lock(void);
extern void device_links_read_unlock(int idx);
+extern int device_links_read_lock_held(void);
extern int device_links_check_suppliers(struct device *dev);
extern void device_links_driver_bound(struct device *dev);
extern void device_links_driver_cleanup(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index fd7511e04e62..6c5ca9685647 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -68,6 +68,11 @@ void device_links_read_unlock(int idx)
{
srcu_read_unlock(&device_links_srcu, idx);
}
+
+int device_links_read_lock_held(void)
+{
+ return srcu_read_lock_held(&device_links_srcu);
+}
#else /* !CONFIG_SRCU */
static DECLARE_RWSEM(device_links_lock);
@@ -91,6 +96,11 @@ void device_links_read_unlock(int not_used)
{
up_read(&device_links_lock);
}
+
+int device_links_read_lock_held(void)
+{
+ return lock_is_held(&device_links_lock);
+}
#endif /* !CONFIG_SRCU */
/**
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 952a1e7057c7..7a10e8379a70 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -287,7 +287,8 @@ static int rpm_get_suppliers(struct device *dev)
{
struct device_link *link;
- list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held()) {
int retval;
if (!(link->flags & DL_FLAG_PM_RUNTIME) ||
@@ -309,7 +310,8 @@ static void rpm_put_suppliers(struct device *dev)
{
struct device_link *link;
- list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held()) {
if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
continue;
@@ -1640,7 +1642,8 @@ void pm_runtime_clean_up_links(struct device *dev)
idx = device_links_read_lock();
- list_for_each_entry_rcu(link, &dev->links.consumers, s_node) {
+ list_for_each_entry_rcu(link, &dev->links.consumers, s_node,
+ device_links_read_lock_held()) {
if (link->flags & DL_FLAG_STATELESS)
continue;
@@ -1662,7 +1665,8 @@ void pm_runtime_get_suppliers(struct device *dev)
idx = device_links_read_lock();
- list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held())
if (link->flags & DL_FLAG_PM_RUNTIME) {
link->supplier_preactivated = true;
refcount_inc(&link->rpm_active);
@@ -1683,7 +1687,8 @@ void pm_runtime_put_suppliers(struct device *dev)
idx = device_links_read_lock();
- list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held())
if (link->supplier_preactivated) {
link->supplier_preactivated = false;
if (refcount_dec_not_one(&link->rpm_active))
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
` (2 preceding siblings ...)
2019-06-01 22:27 ` [RFC 3/6] driver/core: Convert to use built-in RCU list checking Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
2019-06-05 1:24 ` Daniel Jordan
2019-06-01 22:27 ` [RFC 5/6] x86/pci: Pass lockdep condition to pcm_mmcfg_list iterator Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 6/6] acpi: Use built-in RCU list checking for acpi_ioremaps list Joel Fernandes (Google)
5 siblings, 1 reply; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
list_for_each_entry_rcu now has support to check for RCU reader sections
as well as lock. Just use the support in it, instead of explictly
checking in the caller.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/workqueue.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 9657315405de..91ed7aca16e5 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -424,9 +424,8 @@ static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
* ignored.
*/
#define for_each_pwq(pwq, wq) \
- list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node) \
- if (({ assert_rcu_or_wq_mutex(wq); false; })) { } \
- else
+ list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node, \
+ lock_is_held(&(wq->mutex).dep_map))
#ifdef CONFIG_DEBUG_OBJECTS_WORK
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check
2019-06-01 22:27 ` [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check Joel Fernandes (Google)
@ 2019-06-05 1:24 ` Daniel Jordan
2019-06-05 13:04 ` Joel Fernandes
0 siblings, 1 reply; 21+ messages in thread
From: Daniel Jordan @ 2019-06-05 1:24 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Sat, Jun 01, 2019 at 06:27:36PM -0400, Joel Fernandes (Google) wrote:
> list_for_each_entry_rcu now has support to check for RCU reader sections
> as well as lock. Just use the support in it, instead of explictly
> checking in the caller.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> kernel/workqueue.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 9657315405de..91ed7aca16e5 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -424,9 +424,8 @@ static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
> * ignored.
> */
> #define for_each_pwq(pwq, wq) \
> - list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node) \
> - if (({ assert_rcu_or_wq_mutex(wq); false; })) { } \
> - else
> + list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node, \
> + lock_is_held(&(wq->mutex).dep_map))
>
I think the definition of assert_rcu_or_wq_mutex can also be deleted.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check
2019-06-05 1:24 ` Daniel Jordan
@ 2019-06-05 13:04 ` Joel Fernandes
0 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes @ 2019-06-05 13:04 UTC (permalink / raw)
To: Daniel Jordan
Cc: LKML, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, Eric Dumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
Kees Cook, Kernel Hardening, Lai Jiangshan, Len Brown,
linux-acpi, linux-pci, Linux PM, Mathieu Desnoyers, Neil Brown,
netdev, Oleg Nesterov, Paul E. McKenney, Pavel Machek,
Peter Zilstra, Rafael J. Wysocki, rcu, Steven Rostedt, Tejun Heo,
Thomas Gleixner, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Tue, Jun 4, 2019 at 9:25 PM Daniel Jordan <daniel.m.jordan@oracle.com> wrote:
>
> On Sat, Jun 01, 2019 at 06:27:36PM -0400, Joel Fernandes (Google) wrote:
> > list_for_each_entry_rcu now has support to check for RCU reader sections
> > as well as lock. Just use the support in it, instead of explictly
> > checking in the caller.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > kernel/workqueue.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index 9657315405de..91ed7aca16e5 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -424,9 +424,8 @@ static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
> > * ignored.
> > */
> > #define for_each_pwq(pwq, wq) \
> > - list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node) \
> > - if (({ assert_rcu_or_wq_mutex(wq); false; })) { } \
> > - else
> > + list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node, \
> > + lock_is_held(&(wq->mutex).dep_map))
> >
>
> I think the definition of assert_rcu_or_wq_mutex can also be deleted.
Sure, will do. Thank you.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 5/6] x86/pci: Pass lockdep condition to pcm_mmcfg_list iterator
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
` (3 preceding siblings ...)
2019-06-01 22:27 ` [RFC 4/6] workqueue: Convert for_each_wq to use built-in list check Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
2019-06-01 22:27 ` [RFC 6/6] acpi: Use built-in RCU list checking for acpi_ioremaps list Joel Fernandes (Google)
5 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
The pcm_mmcfg_list is traversed with list_for_each_entry_rcu without a
reader-lock held, because the pci_mmcfg_lock is already held. Make this
known to the list macro so that it fixes new lockdep warnings that
trigger due to lockdep checks added to list_for_each_entry_rcu().
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
arch/x86/pci/mmconfig-shared.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 7389db538c30..6fa42e9c4e6f 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -29,6 +29,7 @@
static bool pci_mmcfg_running_state;
static bool pci_mmcfg_arch_init_failed;
static DEFINE_MUTEX(pci_mmcfg_lock);
+#define pci_mmcfg_lock_held() lock_is_held(&(pci_mmcfg_lock).dep_map)
LIST_HEAD(pci_mmcfg_list);
@@ -54,7 +55,7 @@ static void list_add_sorted(struct pci_mmcfg_region *new)
struct pci_mmcfg_region *cfg;
/* keep list sorted by segment and starting bus number */
- list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
+ list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list, pci_mmcfg_lock_held()) {
if (cfg->segment > new->segment ||
(cfg->segment == new->segment &&
cfg->start_bus >= new->start_bus)) {
@@ -118,7 +119,7 @@ struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
{
struct pci_mmcfg_region *cfg;
- list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list)
+ list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list, pci_mmcfg_lock_held())
if (cfg->segment == segment &&
cfg->start_bus <= bus && bus <= cfg->end_bus)
return cfg;
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC 6/6] acpi: Use built-in RCU list checking for acpi_ioremaps list
2019-06-01 22:27 [RFC 0/6] Harden list_for_each_entry_rcu() and family Joel Fernandes (Google)
` (4 preceding siblings ...)
2019-06-01 22:27 ` [RFC 5/6] x86/pci: Pass lockdep condition to pcm_mmcfg_list iterator Joel Fernandes (Google)
@ 2019-06-01 22:27 ` Joel Fernandes (Google)
5 siblings, 0 replies; 21+ messages in thread
From: Joel Fernandes (Google) @ 2019-06-01 22:27 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
David S. Miller, edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI,
H. Peter Anvin, Ingo Molnar, Josh Triplett, keescook,
kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki, rcu,
Steven Rostedt, Tejun Heo, Thomas Gleixner,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
list_for_each_entry_rcu has built-in RCU and lock checking. Make use of
it for acpi_ioremaps list traversal.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
drivers/acpi/osl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f29e427d0d1d..c8b5d712c7ae 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/mm.h>
#include <linux/highmem.h>
+#include <linux/lockdep.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
#include <linux/kmod.h>
@@ -94,6 +95,7 @@ struct acpi_ioremap {
static LIST_HEAD(acpi_ioremaps);
static DEFINE_MUTEX(acpi_ioremap_lock);
+#define acpi_ioremap_lock_held() lock_is_held(&acpi_ioremap_lock.dep_map)
static void __init acpi_request_region (struct acpi_generic_address *gas,
unsigned int length, char *desc)
@@ -220,7 +222,7 @@ acpi_map_lookup(acpi_physical_address phys, acpi_size size)
{
struct acpi_ioremap *map;
- list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+ list_for_each_entry_rcu(map, &acpi_ioremaps, list, acpi_ioremap_lock_held())
if (map->phys <= phys &&
phys + size <= map->phys + map->size)
return map;
@@ -263,7 +265,7 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
{
struct acpi_ioremap *map;
- list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+ list_for_each_entry_rcu(map, &acpi_ioremaps, list, acpi_ioremap_lock_held())
if (map->virt <= virt &&
virt + size <= map->virt + map->size)
return map;
--
2.22.0.rc1.311.g5d7573a151-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread