Skip to content

Commit e81baf4

Browse files
Sebastian Andrzej SiewiorPaul E. McKenney
Sebastian Andrzej Siewior
authored and
Paul E. McKenney
committed
srcu: Remove srcu_queue_delayed_work_on()
srcu_queue_delayed_work_on() disables preemption (and therefore CPU hotplug in RCU's case) and then checks based on its own accounting if a CPU is online. If the CPU is online it uses queue_delayed_work_on() otherwise it fallbacks to queue_delayed_work(). The problem here is that queue_work() on -RT does not work with disabled preemption. queue_work_on() works also on an offlined CPU. queue_delayed_work_on() has the problem that it is possible to program a timer on an offlined CPU. This timer will fire once the CPU is online again. But until then, the timer remains programmed and nothing will happen. Add a local timer which will fire (as requested per delay) on the local CPU and then enqueue the work on the specific CPU. RCUtorture testing with SRCU-P for 24h showed no problems. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
1 parent c8ca1aa commit e81baf4

File tree

4 files changed

+26
-44
lines changed

4 files changed

+26
-44
lines changed

‎include/linux/srcutree.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ struct srcu_data {
4545
unsigned long srcu_gp_seq_needed; /* Furthest future GP needed. */
4646
unsigned long srcu_gp_seq_needed_exp; /* Furthest future exp GP. */
4747
bool srcu_cblist_invoking; /* Invoking these CBs? */
48-
struct delayed_work work; /* Context for CB invoking. */
48+
struct timer_list delay_work; /* Delay for CB invoking */
49+
struct work_struct work; /* Context for CB invoking. */
4950
struct rcu_head srcu_barrier_head; /* For srcu_barrier() use. */
5051
struct srcu_node *mynode; /* Leaf srcu_node. */
5152
unsigned long grpmask; /* Mask for leaf srcu_node */

‎kernel/rcu/srcutree.c

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ static bool __read_mostly srcu_init_done;
5858
static void srcu_invoke_callbacks(struct work_struct *work);
5959
static void srcu_reschedule(struct srcu_struct *ssp, unsigned long delay);
6060
static void process_srcu(struct work_struct *work);
61+
static void srcu_delay_timer(struct timer_list *t);
6162

6263
/* Wrappers for lock acquisition and release, see raw_spin_lock_rcu_node(). */
6364
#define spin_lock_rcu_node(p) \
@@ -156,7 +157,8 @@ static void init_srcu_struct_nodes(struct srcu_struct *ssp, bool is_static)
156157
snp->grphi = cpu;
157158
}
158159
sdp->cpu = cpu;
159-
INIT_DELAYED_WORK(&sdp->work, srcu_invoke_callbacks);
160+
INIT_WORK(&sdp->work, srcu_invoke_callbacks);
161+
timer_setup(&sdp->delay_work, srcu_delay_timer, 0);
160162
sdp->ssp = ssp;
161163
sdp->grpmask = 1 << (cpu - sdp->mynode->grplo);
162164
if (is_static)
@@ -386,13 +388,19 @@ void _cleanup_srcu_struct(struct srcu_struct *ssp, bool quiesced)
386388
} else {
387389
flush_delayed_work(&ssp->work);
388390
}
389-
for_each_possible_cpu(cpu)
391+
for_each_possible_cpu(cpu) {
392+
struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
393+
390394
if (quiesced) {
391-
if (WARN_ON(delayed_work_pending(&per_cpu_ptr(ssp->sda, cpu)->work)))
395+
if (WARN_ON(timer_pending(&sdp->delay_work)))
396+
return; /* Just leak it! */
397+
if (WARN_ON(work_pending(&sdp->work)))
392398
return; /* Just leak it! */
393399
} else {
394-
flush_delayed_work(&per_cpu_ptr(ssp->sda, cpu)->work);
400+
del_timer_sync(&sdp->delay_work);
401+
flush_work(&sdp->work);
395402
}
403+
}
396404
if (WARN_ON(rcu_seq_state(READ_ONCE(ssp->srcu_gp_seq)) != SRCU_STATE_IDLE) ||
397405
WARN_ON(srcu_readers_active(ssp))) {
398406
pr_info("%s: Active srcu_struct %p state: %d\n",
@@ -463,39 +471,23 @@ static void srcu_gp_start(struct srcu_struct *ssp)
463471
WARN_ON_ONCE(state != SRCU_STATE_SCAN1);
464472
}
465473

466-
/*
467-
* Track online CPUs to guide callback workqueue placement.
468-
*/
469-
DEFINE_PER_CPU(bool, srcu_online);
470474

471-
void srcu_online_cpu(unsigned int cpu)
475+
static void srcu_delay_timer(struct timer_list *t)
472476
{
473-
WRITE_ONCE(per_cpu(srcu_online, cpu), true);
474-
}
477+
struct srcu_data *sdp = container_of(t, struct srcu_data, delay_work);
475478

476-
void srcu_offline_cpu(unsigned int cpu)
477-
{
478-
WRITE_ONCE(per_cpu(srcu_online, cpu), false);
479+
queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
479480
}
480481

481-
/*
482-
* Place the workqueue handler on the specified CPU if online, otherwise
483-
* just run it whereever. This is useful for placing workqueue handlers
484-
* that are to invoke the specified CPU's callbacks.
485-
*/
486-
static bool srcu_queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
487-
struct delayed_work *dwork,
482+
static void srcu_queue_delayed_work_on(struct srcu_data *sdp,
488483
unsigned long delay)
489484
{
490-
bool ret;
485+
if (!delay) {
486+
queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
487+
return;
488+
}
491489

492-
preempt_disable();
493-
if (READ_ONCE(per_cpu(srcu_online, cpu)))
494-
ret = queue_delayed_work_on(cpu, wq, dwork, delay);
495-
else
496-
ret = queue_delayed_work(wq, dwork, delay);
497-
preempt_enable();
498-
return ret;
490+
timer_reduce(&sdp->delay_work, jiffies + delay);
499491
}
500492

501493
/*
@@ -504,7 +496,7 @@ static bool srcu_queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
504496
*/
505497
static void srcu_schedule_cbs_sdp(struct srcu_data *sdp, unsigned long delay)
506498
{
507-
srcu_queue_delayed_work_on(sdp->cpu, rcu_gp_wq, &sdp->work, delay);
499+
srcu_queue_delayed_work_on(sdp, delay);
508500
}
509501

510502
/*
@@ -1186,7 +1178,8 @@ static void srcu_invoke_callbacks(struct work_struct *work)
11861178
struct srcu_data *sdp;
11871179
struct srcu_struct *ssp;
11881180

1189-
sdp = container_of(work, struct srcu_data, work.work);
1181+
sdp = container_of(work, struct srcu_data, work);
1182+
11901183
ssp = sdp->ssp;
11911184
rcu_cblist_init(&ready_cbs);
11921185
spin_lock_irq_rcu_node(sdp);

‎kernel/rcu/tree.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3408,8 +3408,6 @@ int rcutree_online_cpu(unsigned int cpu)
34083408
raw_spin_lock_irqsave_rcu_node(rnp, flags);
34093409
rnp->ffmask |= rdp->grpmask;
34103410
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
3411-
if (IS_ENABLED(CONFIG_TREE_SRCU))
3412-
srcu_online_cpu(cpu);
34133411
if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE)
34143412
return 0; /* Too early in boot for scheduler work. */
34153413
sync_sched_exp_online_cleanup(cpu);
@@ -3434,8 +3432,6 @@ int rcutree_offline_cpu(unsigned int cpu)
34343432
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
34353433

34363434
rcutree_affinity_setting(cpu, cpu);
3437-
if (IS_ENABLED(CONFIG_TREE_SRCU))
3438-
srcu_offline_cpu(cpu);
34393435
return 0;
34403436
}
34413437

‎kernel/rcu/tree.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,3 @@ static void rcu_bind_gp_kthread(void);
458458
static bool rcu_nohz_full_cpu(void);
459459
static void rcu_dynticks_task_enter(void);
460460
static void rcu_dynticks_task_exit(void);
461-
462-
#ifdef CONFIG_SRCU
463-
void srcu_online_cpu(unsigned int cpu);
464-
void srcu_offline_cpu(unsigned int cpu);
465-
#else /* #ifdef CONFIG_SRCU */
466-
void srcu_online_cpu(unsigned int cpu) { }
467-
void srcu_offline_cpu(unsigned int cpu) { }
468-
#endif /* #else #ifdef CONFIG_SRCU */

0 commit comments

Comments
 (0)