| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8714bdf6-e257-446c-855f-0f4e65e2921e@paulmck-laptop>
Date: Thu, 7 Nov 2024 13:53:34 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Boqun Feng <boqun.feng@...il.com>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, Marco Elver <elver@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Tomas Gleixner <tglx@...utronix.de>,
Vlastimil Babka <vbabka@...e.cz>, akpm@...ux-foundation.org,
cl@...ux.com, iamjoonsoo.kim@....com, longman@...hat.com,
penberg@...nel.org, rientjes@...gle.com, sfr@...b.auug.org.au
Subject: Re: [PATCH v2 3/3] scftorture: Use a lock-less list to free memory.
On Thu, Nov 07, 2024 at 12:45:25PM -0800, Boqun Feng wrote:
> On Thu, Nov 07, 2024 at 12:13:08PM +0100, Sebastian Andrzej Siewior wrote:
> > scf_handler() is used as a SMP function call. This function is always
> > invoked in IRQ-context even with forced-threading enabled. This function
> > frees memory which not allowed on PREEMPT_RT because the locking
> > underneath is using sleeping locks.
> >
> > Add a per-CPU scf_free_pool where each SMP functions adds its memory to
> > be freed. This memory is then freed by scftorture_invoker() on each
> > iteration. On the majority of invocations the number of items is less
> > than five. If the thread sleeps/ gets delayed the number exceed 350 but
> > did not reach 400 in testing. These were the spikes during testing.
> > The bulk free of 64 pointers at once should improve the give-back if the
> > list grows. The list size is ~1.3 items per invocations.
> >
> > Having one global scf_free_pool with one cleaning thread let the list
> > grow to over 10.000 items with 32 CPUs (again, spikes not the average)
> > especially if the CPU went to sleep. The per-CPU part looks like a good
> > compromise.
> >
> > Reported-by: "Paul E. McKenney" <paulmck@...nel.org>
> > Closes: https://lore.kernel.org/lkml/41619255-cdc2-4573-a360-7794fc3614f7@paulmck-laptop/
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> > ---
> > kernel/scftorture.c | 39 +++++++++++++++++++++++++++++++++++----
> > 1 file changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/scftorture.c b/kernel/scftorture.c
> > index 555b3b10621fe..1268a91af5d88 100644
> > --- a/kernel/scftorture.c
> > +++ b/kernel/scftorture.c
> > @@ -97,6 +97,7 @@ struct scf_statistics {
> > static struct scf_statistics *scf_stats_p;
> > static struct task_struct *scf_torture_stats_task;
> > static DEFINE_PER_CPU(long long, scf_invoked_count);
> > +static DEFINE_PER_CPU(struct llist_head, scf_free_pool);
> >
> > // Data for random primitive selection
> > #define SCF_PRIM_RESCHED 0
> > @@ -133,6 +134,7 @@ struct scf_check {
> > bool scfc_wait;
> > bool scfc_rpc;
> > struct completion scfc_completion;
> > + struct llist_node scf_node;
> > };
> >
> > // Use to wait for all threads to start.
> > @@ -148,6 +150,31 @@ static DEFINE_TORTURE_RANDOM_PERCPU(scf_torture_rand);
> >
> > extern void resched_cpu(int cpu); // An alternative IPI vector.
> >
> > +static void scf_add_to_free_list(struct scf_check *scfcp)
> > +{
> > + struct llist_head *pool;
> > + unsigned int cpu;
> > +
> > + cpu = raw_smp_processor_id() % nthreads;
> > + pool = &per_cpu(scf_free_pool, cpu);
> > + llist_add(&scfcp->scf_node, pool);
> > +}
> > +
> > +static void scf_cleanup_free_list(unsigned int cpu)
> > +{
> > + struct llist_head *pool;
> > + struct llist_node *node;
> > + struct scf_check *scfcp;
> > +
> > + pool = &per_cpu(scf_free_pool, cpu);
> > + node = llist_del_all(pool);
> > + while (node) {
> > + scfcp = llist_entry(node, struct scf_check, scf_node);
> > + node = node->next;
> > + kfree(scfcp);
> > + }
> > +}
> > +
> > // Print torture statistics. Caller must ensure serialization.
> > static void scf_torture_stats_print(void)
> > {
> > @@ -296,7 +323,7 @@ static void scf_handler(void *scfc_in)
> > if (scfcp->scfc_rpc)
> > complete(&scfcp->scfc_completion);
> > } else {
> > - kfree(scfcp);
> > + scf_add_to_free_list(scfcp);
> > }
> > }
> >
> > @@ -363,7 +390,7 @@ static void scftorture_invoke_one(struct scf_statistics *scfp, struct torture_ra
> > scfp->n_single_wait_ofl++;
> > else
> > scfp->n_single_ofl++;
> > - kfree(scfcp);
> > + scf_add_to_free_list(scfcp);
> > scfcp = NULL;
> > }
> > break;
> > @@ -391,7 +418,7 @@ static void scftorture_invoke_one(struct scf_statistics *scfp, struct torture_ra
> > preempt_disable();
> > } else {
> > scfp->n_single_rpc_ofl++;
> > - kfree(scfcp);
> > + scf_add_to_free_list(scfcp);
> > scfcp = NULL;
> > }
> > break;
> > @@ -428,7 +455,7 @@ static void scftorture_invoke_one(struct scf_statistics *scfp, struct torture_ra
> > pr_warn("%s: Memory-ordering failure, scfs_prim: %d.\n", __func__, scfsp->scfs_prim);
> > atomic_inc(&n_mb_out_errs); // Leak rather than trash!
> > } else {
> > - kfree(scfcp);
> > + scf_add_to_free_list(scfcp);
> > }
> > barrier(); // Prevent race-reduction compiler optimizations.
> > }
> > @@ -479,6 +506,8 @@ static int scftorture_invoker(void *arg)
> > VERBOSE_SCFTORTOUT("scftorture_invoker %d started", scfp->cpu);
> >
> > do {
> > + scf_cleanup_free_list(cpu);
> > +
> > scftorture_invoke_one(scfp, &rand);
> > while (cpu_is_offline(cpu) && !torture_must_stop()) {
> > schedule_timeout_interruptible(HZ / 5);
> > @@ -538,6 +567,8 @@ static void scf_torture_cleanup(void)
> >
> > end:
> > torture_cleanup_end();
> > + for (i = 0; i < nthreads; i++)
>
> This needs to be:
>
> for (i = 0; i < nr_cpu_ids; i++)
>
> because nthreads can be larger than nr_cpu_ids, and it'll access a
> out-of-bound percpu section.
I clearly did not test thoroughly enough. Good catch!!!
Thanx, Paul
> Regards,
> Boqun
>
> > + scf_cleanup_free_list(i);
> > }
> >
> > static int __init scf_torture_init(void)
> > --
> > 2.45.2
> >
Powered by blists - more mailing lists