linux-kernel - Re: [PATCH] driver : staging : ion: optimization for decreasing memory fragmentaion

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: <CAGWkznH3Be5MSJi7_=Eoauf1=yZHaCTR4HL-gQH7_TORNEtzeQ@mail.gmail.com>
Date:   Wed, 20 Mar 2019 14:11:49 +0800
From:   Zhaoyang Huang <huangzhaoyang@...il.com>
To:     David Rientjes <rientjes@...gle.com>
Cc:     Chintan Pandya <cpandya@...eaurora.org>,
        Joe Perches <joe@...ches.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] driver : staging : ion: optimization for decreasing
 memory fragmentaion

On Wed, Mar 20, 2019 at 9:10 AM David Rientjes <rientjes@...gle.com> wrote:
>
> On Thu, 14 Mar 2019, Zhaoyang Huang wrote:
>
> > From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> >
> > Two action for this patch:
> > 1. set a batch size for system heap's shrinker, which can have it buffer
> > reasonable page blocks in pool for future allocation.
> > 2. reverse the order sequence when free page blocks, the purpose is also
> > to have system heap keep as more big blocks as it can.
> >
> > By testing on an android system with 2G RAM, the changes with setting
> > batch = 48MB can help reduce the fragmentation obviously and improve
> > big block allocation speed for 15%.
> >
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> > ---
> >  drivers/staging/android/ion/ion_heap.c        | 12 +++++++++++-
> >  drivers/staging/android/ion/ion_system_heap.c |  2 +-
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
> > index 31db510..9e9caf2 100644
> > --- a/drivers/staging/android/ion/ion_heap.c
> > +++ b/drivers/staging/android/ion/ion_heap.c
> > @@ -16,6 +16,8 @@
> >  #include <linux/vmalloc.h>
> >  #include "ion.h"
> >
> > +unsigned long ion_heap_batch = 0;
>
> static?
ok
>
> > +
> >  void *ion_heap_map_kernel(struct ion_heap *heap,
> >                         struct ion_buffer *buffer)
> >  {
> > @@ -303,7 +305,15 @@ int ion_heap_init_shrinker(struct ion_heap *heap)
> >       heap->shrinker.count_objects = ion_heap_shrink_count;
> >       heap->shrinker.scan_objects = ion_heap_shrink_scan;
> >       heap->shrinker.seeks = DEFAULT_SEEKS;
> > -     heap->shrinker.batch = 0;
> > +     heap->shrinker.batch = ion_heap_batch;
> >
> >       return register_shrinker(&heap->shrinker);
> >  }
> > +
> > +static int __init ion_system_heap_batch_init(char *arg)
> > +{
> > +      ion_heap_batch = memparse(arg, NULL);
> > +
>
> No bounds checking?  What are the legitimate upper and lower bounds here?
Actruly, ion_heap_batch will work during shrink_slab, which shown bellow.
We can find that it is hard that to set batch_size as a constant value
as total ram size is different to each system. Furthermore, it is also
no need to set a percentage thing, "total_scan >= freeable" will work
as another threshold of slab size.
...
while (total_scan >= batch_size ||
       total_scan >= freeable) {
    unsigned long nr_to_scan = min(batch_size, total_scan);
    ret = shrinker->scan_objects(shrinker, shrinkctl);
...
shrinkctl->nr_to_scan = nr_to_scan;
shrinkctl->nr_scanned = nr_to_scan;
ret = shrinker->scan_objects(shrinker, shrinkctl);
>
> > +     return 0;
> > +}
> > +early_param("ion_batch", ion_system_heap_batch_init);
> > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
> > index 701eb9f..d249f8d 100644
> > --- a/drivers/staging/android/ion/ion_system_heap.c
> > +++ b/drivers/staging/android/ion/ion_system_heap.c
> > @@ -182,7 +182,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask,
> >       if (!nr_to_scan)
> >               only_scan = 1;
> >
> > -     for (i = 0; i < NUM_ORDERS; i++) {
> > +     for (i = NUM_ORDERS - 1; i >= 0; i--) {
> >               pool = sys_heap->pools[i];
> >
> >               if (only_scan) {
>
> Can we get a Documentation update on how we can use ion_batch and what the
> appropriate settings are (and in what circumstances)?
ok, I will explain it here firstly.
ion_heap_batch will work as the batch_size during shink_slab, which
help the heap buffer some of the page blocks for further allocation.
My test is based on a android system with 2G RAM. We find that
multimedia related cases is the chief consumer of the ion system heap
and cause memory fragmentation after a period of running. By
configuring ion_heap_batch as 48M(3 x camera peak consuming value) and
revert the shrink order, we can almost eliminate such scenario during
the test and improve the allocating speed up to 15%.
For common policy, the batch size should depend on the practical
scenario. The peak value can be got via sysfs or kernel log.