Re: [PATCH 04/13] tracing: Rename kvfree_rcu() to kvfree_rcu_mightsleep()

[Uladzislau Rezki <urezki@gmail.com>]

On Thu, Mar 16, 2023 at 04:05:09PM +0100, Uladzislau Rezki wrote:
> On Thu, Mar 16, 2023 at 09:56:53AM -0400, Steven Rostedt wrote:
> > On Thu, 16 Mar 2023 09:16:37 +0100
> > Uladzislau Rezki <urezki@gmail.com> wrote:
> > 
> > > > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > > > index ef8ed3b65d05..e6037752dcf0 100644
> > > > --- a/kernel/trace/trace_probe.h
> > > > +++ b/kernel/trace/trace_probe.h
> > > > @@ -256,6 +256,7 @@ struct trace_probe {
> > > >  struct event_file_link {
> > > >  	struct trace_event_file		*file;
> > > >  	struct list_head		list;
> > > > +	struct rcu_head			rcu;
> > > >  };
> > > >  
> > > >  static inline bool trace_probe_test_flag(struct trace_probe *tp,
> > > >  
> > > struct foo_a {
> > >   int a;
> > >   int b;
> > > };
> > 
> > Most machines today are 64 bits, even low end machines.
> > 
> >  struct foo_a {
> > 	long long a;
> > 	long long b;
> >  };
> > 
> > is more accurate. That's 16 bytes.
> > 
> > Although it is more likely off because list_head is a double pointer. But
> > let's just go with this, as the amount really doesn't matter here.
> > 
> > > 
> > > your obj size is 8 byte
> > > 
> > > struct foo_b {
> > >   struct rcu_head rcu;
> > 
> > Isn't rcu_head defined as;
> > 
> > struct callback_head {
> >         struct callback_head *next;
> >         void (*func)(struct callback_head *head);
> > } __attribute__((aligned(sizeof(void *))));
> > #define rcu_head callback_head
> > 
> > Which makes it 8 not 16 on 32 bit as well?
> > 
> > >   int a;
> > >   int b;
> > > };
> > 
> > So it should be 8 + 8 = 16, on 32 bit and 16 + 16 = 32 on 64bit.
> > 
> > > 
> > > now it becomes 16 + 8 = 24 bytes. In reallity a foo_b object
> > > will be 32 bytes since there is no slab for 24 bytes:
> > > 
> > > <snip>
> > >   kmalloc-32         19840  19840     32  128    1 : tunables    0    0    0 : slabdata    155    155      0
> > >   kmalloc-16         28857  28928     16  256    1 : tunables    0    0    0 : slabdata    113    113      0
> > >   kmalloc-8          37376  37376      8  512    1 : tunables    0    0    0 : slabdata     73     73      0
> > > <snip>
> > > 
> > > if we allocate 512 objects of foo_a it would be 4096 bytes
> > > in case of foo_b it is 24 * 512 = 12228 bytes.
> > 
> > This is for probe events. We usually allocate 1, maybe 2. Oh, some may even
> > allocate 100 to be crazy. But each probe event is in reality much larger
> > (1K perhaps) as each one allocates dentry's, inodes, etc. So 8 or 16 bytes
> > extra is still lost in the noise.
> > 
> > > 
> > > single argument will give you 4096 + 512 * 8 = 8192 bytes
> > > int terms of memory consumtion.
> > 
> > If someone allocate 512 instances, that would be closer to a meg in size
> > without this change. 8k is probably less than 1%
> > 
> In percentage. My case. (12228 - 8192) * 100 / 12228 = ~33% difference.
> 
> > > 
> > > And double argument will not give you better performance comparing
> > > with a single argument.
> > 
> > It will, because it will no longer have to allocate anything if need be.
> > Note, when it doesn't allocate the system is probably mostly idle and we
> > don't care about performance, but when it needs allocation, that's likely a
> > time when performance is a bit more important.
> > 
> The problem further is about pointer chasing, like comparing arrays and
> lists. It will take longer time to offload all pointers.
> 
Since i have a data, IMHO it is better to share than not:

--bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=3 rcuscale.holdoff=20 rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot"

# double-argument 10 run
Total time taken by all kfree'ers: 4387872408 ns, loops: 10000, batches: 958,  memory footprint: 40MB
Total time taken by all kfree'ers: 4415232304 ns, loops: 10000, batches: 982,  memory footprint: 39MB
Total time taken by all kfree'ers: 4270303081 ns, loops: 10000, batches: 955,  memory footprint: 42MB
Total time taken by all kfree'ers: 4364984147 ns, loops: 10000, batches: 953,  memory footprint: 40MB
Total time taken by all kfree'ers: 4225994506 ns, loops: 10000, batches: 942,  memory footprint: 40MB
Total time taken by all kfree'ers: 4601087346 ns, loops: 10000, batches: 1033, memory footprint: 40MB
Total time taken by all kfree'ers: 4853397855 ns, loops: 10000, batches: 1109, memory footprint: 38MB
Total time taken by all kfree'ers: 4627914204 ns, loops: 10000, batches: 1037, memory footprint: 39MB
Total time taken by all kfree'ers: 4274587317 ns, loops: 10000, batches: 938,  memory footprint: 33MB
Total time taken by all kfree'ers: 4642151205 ns, loops: 10000, batches: 1068, memory footprint: 39MB

# single-argument 10 run
Total time taken by all kfree'ers: 3661190052 ns, loops: 10000, batches: 831, memory footprint: 29MB
Total time taken by all kfree'ers: 3616277061 ns, loops: 10000, batches: 780, memory footprint: 27MB
Total time taken by all kfree'ers: 3704584439 ns, loops: 10000, batches: 810, memory footprint: 27MB
Total time taken by all kfree'ers: 3631291959 ns, loops: 10000, batches: 812, memory footprint: 28MB
Total time taken by all kfree'ers: 3610490769 ns, loops: 10000, batches: 795, memory footprint: 27MB
Total time taken by all kfree'ers: 3595446243 ns, loops: 10000, batches: 825, memory footprint: 28MB
Total time taken by all kfree'ers: 3686252889 ns, loops: 10000, batches: 784, memory footprint: 27MB
Total time taken by all kfree'ers: 3821475275 ns, loops: 10000, batches: 869, memory footprint: 27MB
Total time taken by all kfree'ers: 3740407185 ns, loops: 10000, batches: 813, memory footprint: 28MB
Total time taken by all kfree'ers: 3646684795 ns, loops: 10000, batches: 780, memory footprint: 24MB

And yes, there are side effects. For example a low memory condition.

--
Uladzislau Rezki