linux-kernel - Re: [PATCH v3] binder_alloc: Replace kcalloc with kvcalloc to mitigate OOM issues

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZnEO8X-7pjeTFTur@google.com>
Date: Tue, 18 Jun 2024 04:37:05 +0000
From: Carlos Llamas <cmllamas@...gle.com>
To: Lei Liu <liulei.rjpt@...o.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Christian Brauner <brauner@...nel.org>,
	Suren Baghdasaryan <surenb@...gle.com>,
	linux-kernel@...r.kernel.org, opensource.kernel@...o.com
Subject: Re: [PATCH v3] binder_alloc: Replace kcalloc with kvcalloc to
 mitigate OOM issues

On Tue, Jun 18, 2024 at 10:50:17AM +0800, Lei Liu wrote:
> 
> On 2024/6/18 2:43, Carlos Llamas wrote:
> > On Mon, Jun 17, 2024 at 12:01:26PM +0800, Lei Liu wrote:
> > > On 6/15/2024 at 2:38, Carlos Llamas wrote:
> > Yes, all this makes sense. What I don't understand is how "performance
> > of kvcalloc is better". This is not supposed to be.
> 
> Based on my current understanding:
> 1.kvmalloc may allocate memory faster than kmalloc in cases of memory
> fragmentation, which could potentially improve the performance of binder.

I think there is a misunderstanding of the allocations performed in this
benchmark test. Yes, in general when there is heavy memory pressure it
can be faster to use kvmalloc() and not try too hard to reclaim
contiguous memory.

In the case of binder though, this is the mmap() allocation. This call
is part of the "initial setup". In the test, there should only be two
calls to kvmalloc(), since the benchmark is done across two processes.
That's it.

So the time it takes to allocate this memory is irrelevant to the
performance results. Does this make sense?

> 2.Memory allocated by kvmalloc may not be contiguous, which could
> potentially degrade the data read and write speed of binder.

This _is_ what is being considered in the benchmark test instead. There
are repeated accesses to alloc->pages[n]. Your point is then the reason
why I was expecting "same performance at best".

> Hmm, this is really good news. From the current test results, it seems that
> kvmalloc does not degrade performance for binder.

Yeah, not in the "happy" case anyways. I'm not sure what the numbers
look like under some memory pressure.

> I will retest the data on our phone to see if we reach the same conclusion.
> If kvmalloc still proves to be better, we will provide you with the
> reproduction method.
> 
Ok, thanks. I would suggest you do an "adb shell stop" before running
these test. This might help with the noise.

Thanks,
Carlos Llamas