Messages in this thread | | | From | Milton Miller <> | Subject | Re: [PATCH v6 5/8] slub: only IPI CPUs that have per cpu obj to flush | Date | Wed, 11 Jan 2012 01:04:13 -0600 |
| |
On Sun Jan 08 2012 about 11:28:11 EST, Gilad Ben-Yossef wrote: > flush_all() is called for each kmem_cahce_destroy(). So every cache > being destroyed dynamically ended up sending an IPI to each CPU in the
ends up
> system, regardless if the cache has ever been used there. > > For example, if you close the Infinband ipath driver char device file, > the close file ops calls kmem_cache_destroy(). So running some > infiniband config tool on one a single CPU dedicated to system tasks > might interrupt the rest of the 127 CPUs I dedicated to some CPU
127 CPUs dedicated
> intensive task.
CPU intensive or latency sensitive task.
> > I suspect there is a good chance that every line in the output of "git > grep kmem_cache_destroy linux/ | grep '\->'" has a similar scenario. > > This patch attempts to rectify this issue by sending an IPI to flush > the per cpu objects back to the free lists only to CPUs that seems to > have such objects.
that seem to have
> > The check which CPU to IPI is racy but we don't care since asking a > CPU without per cpu objects to flush does no damage and as far as I > can tell the flush_all by itself is racy against allocs on remote > CPUs anyway, so if you meant the flush_all to be determinstic, you
required (vs meant)
> had to arrange for locking regardless. > > Without this patch the following artificial test case: > > $ cd /sys/kernel/slab > $ for DIR in *; do cat $DIR/alloc_calls > /dev/null; done > > produces 166 IPIs on an cpuset isolated CPU. With it it produces none. > > The code path of memory allocation failure for CPUMASK_OFFSTACK=y > config was tested using fault injection framework. > .. > mm/slub.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 09ccee8..31833d6 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2013,9 +2013,17 @@ static void flush_cpu_slab(void *d) > __flush_cpu_slab(s, smp_processor_id()); > } > > +static int has_cpu_slab(int cpu, void *info) > +{ > + struct kmem_cache *s = info; > + struct kmem_cache_cpu *c = per_cpu_ptr(s->cpu_slab, cpu); > + > + return !!(c->page);
__flush_cpu_slab is careful to test that the the per_cpu_ptr is not NULL before referencing the page field. free_percpu likewise ignores NULL pointers. We need to check !!(c && c->page) here.
[change int to bool assuming you make the change in the other patch].
> +} > + > static void flush_all(struct kmem_cache *s) > { > - on_each_cpu(flush_cpu_slab, s, 1); > + on_each_cpu_cond(has_cpu_slab, flush_cpu_slab, s, 1); > }
milton
| |