On Mon, Nov 28, 2022 at 09:20:18AM +0000, Zhang, Qiang1 wrote: > >Subject: [PATCH 2/3] srcu: protect against concurrent srcu_gp_end() > > > >At present, the code chunk "Transition to big if needed" is out of the > >lock srcu_cb_mutex, which allows the following scene: > > > > srcu_gp_end() on cpu1 srcu_gp_end() on cpu2 > > > > init_srcu_struct_nodes() > > ^^ scheduled out in the middle > > init_srcu_struct_nodes() > > ^^ overwrite the context set up by cpu1 > > Hi Pingfan > > This scenario shouldn't happen. > The srcu_gp_end() is invoked only in process_srcu() workfn. > for the same ssp->work, workqueue can guaranteed that the > process_srcu() workfn is executed serially. (view __queue_work () details) > > >How about this: > >In process_one_work() > > set_work_pool_and_clear_pending(work, pool->id); > -----> another ssp->work > can be queued, which can raise the > scenario mentioned above > > worker->current_func(work); > Hi Pingfan The following two code snippets ensure that this scenario does not happen __queue_work(): last_pool = get_work_pool(work); if (last_pool && last_pool != pwq->pool) { struct worker *worker; raw_spin_lock(&last_pool->lock); worker = find_worker_executing_work(last_pool, work); if (worker && worker->current_pwq->wq == wq) { pwq = worker->current_pwq; and process_one_work(): collision = find_worker_executing_work(pool, work); if (unlikely(collision)) { move_linked_works(work, &collision->scheduled, NULL); return; } Thanks Zqiang > >Thanks, > Pingfan > > Thanks > Zqiang > > > > > >Squashing out the race window by putting the code chunk under the > >protection of srcu_cb_mutex. > > > >Signed-off-by: Pingfan Liu <kernelfans@xxxxxxxxx> > >Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx> > >Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx> > >Cc: Frederic Weisbecker <frederic@xxxxxxxxxx> > >Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx> > >Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > >Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> > >Cc: "Zhang, Qiang1" <qiang1.zhang@xxxxxxxxx> > >To: rcu@xxxxxxxxxxxxxxx > >--- > > kernel/rcu/srcutree.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > >diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > >index fe0759d89c2d..281cd69fe804 100644 > >--- a/kernel/rcu/srcutree.c > >+++ b/kernel/rcu/srcutree.c > >@@ -811,6 +811,13 @@ static void srcu_gp_end(struct srcu_struct *ssp) > > spin_unlock_irqrestore_rcu_node(sdp, flags); > > } > > > >+ /* Transition to big if needed. */ > >+ if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) { > >+ if (ss_state == SRCU_SIZE_ALLOC) > >+ init_srcu_struct_nodes(ssp, GFP_KERNEL); > >+ else > >+ smp_store_release(&ssp->srcu_size_state, ss_state + 1); > >+ } > > /* Callback initiation done, allow grace periods after next. */ > > mutex_unlock(&ssp->srcu_cb_mutex); > > > >@@ -826,13 +833,6 @@ static void srcu_gp_end(struct srcu_struct *ssp) > > spin_unlock_irq_rcu_node(ssp); > > } > > > >- /* Transition to big if needed. */ > >- if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) { > >- if (ss_state == SRCU_SIZE_ALLOC) > >- init_srcu_struct_nodes(ssp, GFP_KERNEL); > >- else > >- smp_store_release(&ssp->srcu_size_state, ss_state + 1); > >- } > > } > > > > /* > >-- > >2.31.1 > >