Re: [PATCH] mm/mmu_notifier.c: micro-optimization substitute kzalloc with kmalloc

[Mateusz Nosek]

Hi,

I performed simple benchmarks using custom kernel module with the code 
fragment in question 'copy-pasted' in there in both versions. In case of 
1k, 10k and 100k iterations the average time for kzalloc version was 5.1 
and for kmalloc 3.9, for each iterations number.
The time was measured using 'ktime_get(void)' function and the results 
given here are in ktime_t units.
The machine I use has 4 core Intel(R) Core(TM) i5-3470 CPU @ 3.20GHz CPU.

The performance increase happens, but as you wrote it is probably not 
really noticeable.
I have found 3 other places in kernel code with similar kzalloc related 
issues, none of which seems to be 'hot' code.
I leave the decision if this patch and potential others I would send 
regarding this issue, are worth applying to the community and maintainers.

Best regards,
Mateusz Nosek

On 9/6/2020 4:26 PM, Mike Rapoport wrote:
> Hi,
>
> On Sun, Sep 06, 2020 at 01:43:21PM +0200, mateusznosek0@xxxxxxxxx wrote:
> > From: Mateusz Nosek <mateusznosek0@xxxxxxxxx>
> >
> > Most fields in struct pointed by 'subscriptions' are initialized explicitly
> > after the allocation. By changing kzalloc to kmalloc the call to memset
> > is avoided. As the only new code consists of 2 simple memory accesses,
> > the performance is increased.
>
> Is there a measurable performance increase?
>
> The __mmu_notifier_register() is not used that frequently to trade off
> robustness of kzalloc() for slight (if visible at all) performance gain.
>
> > Signed-off-by: Mateusz Nosek <mateusznosek0@xxxxxxxxx>
> > ---
> >   mm/mmu_notifier.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
> > index 4fc918163dd3..190e198dc5be 100644
> > --- a/mm/mmu_notifier.c
> > +++ b/mm/mmu_notifier.c
> > @@ -625,7 +625,7 @@ int __mmu_notifier_register(struct mmu_notifier *subscription,
> >   		 * know that mm->notifier_subscriptions can't change while we
> >   		 * hold the write side of the mmap_lock.
> >   		 */
> > -		subscriptions = kzalloc(
> > +		subscriptions = kmalloc(
> >   			sizeof(struct mmu_notifier_subscriptions), GFP_KERNEL);
> >   		if (!subscriptions)
> >   			return -ENOMEM;
> > @@ -636,6 +636,8 @@ int __mmu_notifier_register(struct mmu_notifier *subscription,
> >   		subscriptions->itree = RB_ROOT_CACHED;
> >   		init_waitqueue_head(&subscriptions->wq);
> >   		INIT_HLIST_HEAD(&subscriptions->deferred_list);
> > +		subscriptions->active_invalidate_ranges = 0;
> > +		subscriptions->has_itree = false;
> >   	}
> >   
> >   	ret = mm_take_all_locks(mm);
> > --
> > 2.20.1