srcu: Use irq_work to start GP in tiny SRCU

Tiny SRCU's srcu_gp_start_if_needed() directly calls schedule_work(),
which acquires the workqueue pool->lock.

This causes a lockdep splat when call_srcu() is called with a scheduler
lock held, due to:

call_srcu() [holding pi_lock]
srcu_gp_start_if_needed()
schedule_work() -> pool->lock

workqueue_init() / create_worker() [holding pool->lock]
wake_up_process() -> try_to_wake_up() -> pi_lock

Also add irq_work_sync() to cleanup_srcu_struct() to prevent a
use-after-free if a queued irq_work fires after cleanup begins.

Tested with rcutorture SRCU-T and no lockdep warnings.

[ Thanks to Boqun for similar fix in patch "rcu: Use an intermediate irq_work
to start process_srcu()" ]

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun@kernel.org>

authored by

Joel Fernandes and committed by

Boqun Feng 1 month ago a6fc88b2 7c405fb3

+22 -1

2 changed files

expand all

include

linux

srcutiny.h

kernel

rcu

srcutiny.c

include/linux/srcutiny.h

··· 11 11 #ifndef _LINUX_SRCU_TINY_H 12 12 #define _LINUX_SRCU_TINY_H 13 13 14 + #include <linux/irq_work_types.h> 14 15 #include <linux/swait.h> 15 16 16 17 struct srcu_struct { ··· 25 24 struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */ 26 25 struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */ 27 26 struct work_struct srcu_work; /* For driving grace periods. */ 27 + struct irq_work srcu_irq_work; /* Defer schedule_work() to irq work. */ 28 28 #ifdef CONFIG_DEBUG_LOCK_ALLOC 29 29 struct lockdep_map dep_map; 30 30 #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ 31 31 }; 32 32 33 33 void srcu_drive_gp(struct work_struct *wp); 34 + void srcu_tiny_irq_work(struct irq_work *irq_work); 34 35 35 36 #define __SRCU_STRUCT_INIT(name, __ignored, ___ignored, ____ignored) \ 36 37 { \ 37 38 .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \ 38 39 .srcu_cb_tail = &name.srcu_cb_head, \ 39 40 .srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \ 41 + .srcu_irq_work = { .func = srcu_tiny_irq_work }, \ 40 42 __SRCU_DEP_MAP_INIT(name) \ 41 43 } 42 44

+18 -1

kernel/rcu/srcutiny.c

··· 9 9 */ 10 10 11 11 #include <linux/export.h> 12 + #include <linux/irq_work.h> 12 13 #include <linux/mutex.h> 13 14 #include <linux/preempt.h> 14 15 #include <linux/rcupdate_wait.h> ··· 42 41 ssp->srcu_idx_max = 0; 43 42 INIT_WORK(&ssp->srcu_work, srcu_drive_gp); 44 43 INIT_LIST_HEAD(&ssp->srcu_work.entry); 44 + init_irq_work(&ssp->srcu_irq_work, srcu_tiny_irq_work); 45 45 return 0; 46 46 } 47 47 ··· 86 84 void cleanup_srcu_struct(struct srcu_struct *ssp) 87 85 { 88 86 WARN_ON(ssp->srcu_lock_nesting[0] || ssp->srcu_lock_nesting[1]); 87 + irq_work_sync(&ssp->srcu_irq_work); 89 88 flush_work(&ssp->srcu_work); 90 89 WARN_ON(ssp->srcu_gp_running); 91 90 WARN_ON(ssp->srcu_gp_waiting); ··· 180 177 } 181 178 EXPORT_SYMBOL_GPL(srcu_drive_gp); 182 179 180 + /* 181 + * Use an irq_work to defer schedule_work() to avoid acquiring the workqueue 182 + * pool->lock while the caller might hold scheduler locks, causing lockdep 183 + * splats due to workqueue_init() doing a wakeup. 184 + */ 185 + void srcu_tiny_irq_work(struct irq_work *irq_work) 186 + { 187 + struct srcu_struct *ssp; 188 + 189 + ssp = container_of(irq_work, struct srcu_struct, srcu_irq_work); 190 + schedule_work(&ssp->srcu_work); 191 + } 192 + EXPORT_SYMBOL_GPL(srcu_tiny_irq_work); 193 + 183 194 static void srcu_gp_start_if_needed(struct srcu_struct *ssp) 184 195 { 185 196 unsigned long cookie; ··· 206 189 WRITE_ONCE(ssp->srcu_idx_max, cookie); 207 190 if (!READ_ONCE(ssp->srcu_gp_running)) { 208 191 if (likely(srcu_init_done)) 209 - schedule_work(&ssp->srcu_work); 192 + irq_work_queue(&ssp->srcu_irq_work); 210 193 else if (list_empty(&ssp->srcu_work.entry)) 211 194 list_add(&ssp->srcu_work.entry, &srcu_boot_list); 212 195 }

Configure Feed

Configure Feed