On 5 October 2026 01:56:41 BST, Joel Fernandes <[email protected]> wrote: >The writer kthreads keep their state in five parallel arrays indexed by >writer number: writer_tasks, writer_durations, writer_n_durations, >writer_done and writer_freelists. Each must be allocated, NULL-checked, >freed and cleared separately, and the cleanup path needs several >guards to cope with any one of them being missing. > >Collapse them into a single array of struct writer_state, holding the >task pointer, the duration buffer, the measurement count, the done flag >and the existing struct writer_freelist. > >This reduces initialization to one allocation and one out-of-memory >check, and cleanup to a single kfree(). As a side effect, this also >fixes a leak: cleanup only ran its frees under "if (writer_tasks)", so >if the writer_tasks allocation failed while a later allocation (for >example writer_durations or writer_n_durations) still succeeded, those >already-allocated arrays were never freed. With a single allocation, >that partial-allocation state can no longer occur.
All I've gotta say here, is one huge thank you, this cleanup WASS needed. Reviewed-by: Bradley Morgan <[email protected]> Also I ran basic tests: Tested-by: Bradley Morgan <[email protected]> # Power10 > >Assisted-by: Codex >Fixes: 8704baab9bc8 ("rcutorture: Add RCU grace-period performance tests") >Signed-off-by: Joel Fernandes <[email protected]> >--- > kernel/rcu/rcuscale.c | 127 ++++++++++++++++++------------------------ > 1 file changed, 55 insertions(+), 72 deletions(-) > >diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c >index 1097ec15879c..f0bca5dcc754 100644 >--- a/kernel/rcu/rcuscale.c >+++ b/kernel/rcu/rcuscale.c >@@ -115,17 +115,21 @@ struct writer_freelist { > struct writer_mblock *ws_mblocks; > }; > >+struct writer_state { >+ struct writer_freelist wst_fl; >+ struct task_struct *wst_task; >+ u64 *wst_durations; >+ int wst_n_durations; >+ bool wst_done; >+}; >+ > static int nrealreaders; > static int nrealwriters; > static int nrealexp; >-static struct task_struct **writer_tasks; > static struct task_struct **reader_tasks; > static struct task_struct **exp_tasks; > >-static u64 **writer_durations; >-static bool *writer_done; >-static struct writer_freelist *writer_freelists; >-static int *writer_n_durations; >+static struct writer_state *writer_states; > static atomic_t n_rcu_scale_reader_started; > static atomic_t n_rcu_scale_writer_started; > static atomic_t n_rcu_scale_writer_finished; >@@ -504,9 +508,9 @@ static struct writer_mblock *rcu_scale_alloc(long me) > struct writer_freelist *wflp; > struct writer_mblock *wmbp; > >- if (WARN_ON_ONCE(!writer_freelists)) >+ if (WARN_ON_ONCE(!writer_states)) > return NULL; >- wflp = &writer_freelists[me]; >+ wflp = &writer_states[me].wst_fl; > if (llist_empty(&wflp->ws_lhp)) { > // ->ws_lhp is private to its rcu_scale_writer task. > wmbp = container_of(llist_del_all(&wflp->ws_lhg), struct > writer_mblock, wmb_node); >@@ -560,8 +564,9 @@ rcu_scale_writer(void *arg) > u64 t; > DEFINE_TORTURE_RANDOM(tr); > u64 *wdp; >- u64 *wdpp = writer_durations[me]; >- struct writer_freelist *wflp = &writer_freelists[me]; >+ struct writer_state *wsp = &writer_states[me]; >+ u64 *wdpp = wsp->wst_durations; >+ struct writer_freelist *wflp = &wsp->wst_fl; > struct writer_mblock *wmbp = NULL; > > VERBOSE_SCALEOUT_STRING("rcu_scale_writer task started"); >@@ -631,7 +636,7 @@ rcu_scale_writer(void *arg) > started = true; > if (!done && i >= MIN_MEAS && time_after(jiffies, jdone)) { > done = true; >- WRITE_ONCE(writer_done[me], true); >+ WRITE_ONCE(wsp->wst_done, true); > sched_set_normal(current, 0); > pr_alert("%s%s rcu_scale_writer %ld has %d > measurements\n", > scale_type, SCALE_FLAG, me, MIN_MEAS); >@@ -649,7 +654,7 @@ rcu_scale_writer(void *arg) > cur_ops->get_gp_seq(); > } > if (shutdown_secs) { >- writer_tasks[me] = NULL; >+ wsp->wst_task = NULL; > smp_mb(); /* Assign before wake. */ > rcu_scale_cleanup(); > kernel_power_off(); >@@ -665,10 +670,10 @@ rcu_scale_writer(void *arg) > > if (!atomic_xchg(&dumped, 1)) { > for (i = 0; i < nrealwriters; i++) { >- if (writer_done[i]) >+ if (writer_states[i].wst_done) > continue; > pr_info("%s: Task %ld flags writer > %d:\n", __func__, me, i); >- sched_show_task(writer_tasks[i]); >+ >sched_show_task(writer_states[i].wst_task); > } > if (cur_ops->stats) > cur_ops->stats(); >@@ -676,7 +681,7 @@ rcu_scale_writer(void *arg) > } > if (!selfreport && time_after(jiffies, jdone + HZ * (70 + me))) > { > pr_info("%s: Writer %ld self-report: started %d done > %d/%d->%d i %d jdone %lu.\n", >- __func__, me, started, done, writer_done[me], >atomic_read(&n_rcu_scale_writer_finished), i, jiffies - jdone); >+ __func__, me, started, done, wsp->wst_done, >atomic_read(&n_rcu_scale_writer_finished), i, jiffies - jdone); > selfreport = true; > } > if (gp_succeeded && started && !alldone && i < MAX_MEAS - 1) >@@ -687,7 +692,7 @@ rcu_scale_writer(void *arg) > rcu_scale_free(wmbp); > cur_ops->gp_barrier(); > } >- writer_n_durations[me] = i_max + 1; >+ wsp->wst_n_durations = i_max + 1; > torture_kthread_stopping("rcu_scale_writer"); > return 0; > } >@@ -1031,13 +1036,11 @@ rcu_scale_cleanup(void) > reader_tasks = NULL; > } > >- if (writer_tasks) { >+ if (writer_states) { > for (i = 0; i < nrealwriters; i++) { > torture_stop_kthread(rcu_scale_writer, >- writer_tasks[i]); >- if (!writer_n_durations) >- continue; >- j = writer_n_durations[i]; >+ writer_states[i].wst_task); >+ j = writer_states[i].wst_n_durations; > pr_alert("%s%s writer %d gps: %d\n", > scale_type, SCALE_FLAG, i, j); > ngps += j; >@@ -1051,14 +1054,12 @@ rcu_scale_cleanup(void) > rcuscale_seq_diff(b_rcu_gp_test_finished, > b_rcu_gp_test_started)); > for (i = 0; i < nrealwriters; i++) { >- if (!writer_durations) >- break; >- if (!writer_n_durations) >- continue; >- wdpp = writer_durations[i]; >+ struct writer_state *wsp = &writer_states[i]; >+ >+ wdpp = wsp->wst_durations; > if (!wdpp) > continue; >- for (j = 0; j < writer_n_durations[i]; j++) { >+ for (j = 0; j < wsp->wst_n_durations; j++) { > wdp = &wdpp[j]; > pr_alert("%s%s %4d writer-duration: %5d %llu\n", > scale_type, SCALE_FLAG, >@@ -1066,34 +1067,23 @@ rcu_scale_cleanup(void) > if (j % 100 == 0) > schedule_timeout_uninterruptible(1); > } >- kfree(writer_durations[i]); >- if (writer_freelists) { >+ kfree(wdpp); >+ if (wsp->wst_fl.ws_mblocks) { > int ctr = 0; > struct llist_node *llnp; >- struct writer_freelist *wflp = >&writer_freelists[i]; > >- if (wflp->ws_mblocks) { >- llist_for_each(llnp, wflp->ws_lhg.first) >- ctr++; >- llist_for_each(llnp, wflp->ws_lhp.first) >- ctr++; >- WARN_ONCE(ctr != gp_async_max, >- "%s: ctr = %d gp_async_max = >%d\n", >- __func__, ctr, gp_async_max); >- kfree(wflp->ws_mblocks); >- } >+ llist_for_each(llnp, wsp->wst_fl.ws_lhg.first) >+ ctr++; >+ llist_for_each(llnp, wsp->wst_fl.ws_lhp.first) >+ ctr++; >+ WARN_ONCE(ctr != gp_async_max, >+ "%s: ctr = %d gp_async_max = %d\n", >+ __func__, ctr, gp_async_max); >+ kfree(wsp->wst_fl.ws_mblocks); > } > } >- kfree(writer_tasks); >- writer_tasks = NULL; >- kfree(writer_durations); >- writer_durations = NULL; >- kfree(writer_n_durations); >- writer_n_durations = NULL; >- kfree(writer_done); >- writer_done = NULL; >- kfree(writer_freelists); >- writer_freelists = NULL; >+ kfree(writer_states); >+ writer_states = NULL; > } > > /* Do torture-type-specific cleanup operations. */ >@@ -1187,37 +1177,30 @@ rcu_scale_init(void) > goto unwind; > } > } >- writer_tasks = kzalloc_objs(writer_tasks[0], nrealwriters); >- writer_durations = kcalloc(nrealwriters, sizeof(*writer_durations), >GFP_KERNEL); >- writer_n_durations = kzalloc_objs(*writer_n_durations, nrealwriters); >- writer_done = kzalloc_objs(writer_done[0], nrealwriters); >- if (gp_async) { >- if (gp_async_max <= 0) { >- pr_warn("%s: gp_async_max = %d must be greater than >zero.\n", >- __func__, gp_async_max); >- WARN_ON_ONCE(IS_BUILTIN(CONFIG_RCU_TORTURE_TEST)); >- firsterr = -EINVAL; >- goto unwind; >- } >- writer_freelists = kzalloc_objs(writer_freelists[0], >- nrealwriters); >+ writer_states = kzalloc_objs(writer_states[0], nrealwriters); >+ if (gp_async && gp_async_max <= 0) { >+ pr_warn("%s: gp_async_max = %d must be greater than zero.\n", >+ __func__, gp_async_max); >+ WARN_ON_ONCE(IS_BUILTIN(CONFIG_RCU_TORTURE_TEST)); >+ firsterr = -EINVAL; >+ goto unwind; > } >- if (!writer_tasks || !writer_durations || !writer_n_durations || >!writer_done || >- (gp_async && !writer_freelists)) { >+ if (!writer_states) { > SCALEOUT_ERRSTRING("out of memory"); > firsterr = -ENOMEM; > goto unwind; > } > for (i = 0; i < nrealwriters; i++) { >- writer_durations[i] = >- kcalloc(MAX_MEAS, sizeof(*writer_durations[i]), >- GFP_KERNEL); >- if (!writer_durations[i]) { >+ struct writer_state *wsp = &writer_states[i]; >+ >+ wsp->wst_durations = kcalloc(MAX_MEAS, >sizeof(*wsp->wst_durations), >+ GFP_KERNEL); >+ if (!wsp->wst_durations) { > firsterr = -ENOMEM; > goto unwind; > } >- if (writer_freelists) { >- struct writer_freelist *wflp = &writer_freelists[i]; >+ if (gp_async) { >+ struct writer_freelist *wflp = &wsp->wst_fl; > > init_llist_head(&wflp->ws_lhg); > init_llist_head(&wflp->ws_lhp); >@@ -1235,7 +1218,7 @@ rcu_scale_init(void) > } > } > firsterr = torture_create_kthread(rcu_scale_writer, (void *)i, >- writer_tasks[i]); >+ wsp->wst_task); > if (torture_init_error(firsterr)) > goto unwind; > } > --- Thanks! "I'm not a very positive person" - Linus torvalds
