linux-kernel - Re: [PATCH 3/3] zsmalloc: replace kmap_atomic with kmap_local

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] [day] [month] [year] [list]

Message-ID: <CAOuPNLg4YO+6gVtSRYaCzRJTghB5gtPMOAmmBwNeYMH-52zKZQ@mail.gmail.com>
Date: Thu, 3 Oct 2024 23:37:55 +0530
From: Pintu Agarwal <pintu.ping@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Pintu Kumar <quic_pintu@...cinc.com>, minchan@...nel.org, senozhatsky@...omium.org, 
	akpm@...ux-foundation.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	joe@...ches.com, skhan@...uxfoundation.org
Subject: Re: [PATCH 3/3] zsmalloc: replace kmap_atomic with kmap_local_page

Hi Matthew,
Thank you so much for your review and comments.

On Wed, 2 Oct 2024 at 09:08, Matthew Wilcox <willy@...radead.org> wrote:
>
> On Tue, Oct 01, 2024 at 11:23:58PM +0530, Pintu Kumar wrote:
> > @@ -1059,12 +1061,12 @@ static void *__zs_map_object(struct mapping_area *area,
> >       sizes[1] = size - sizes[0];
> >
> >       /* copy object to per-cpu buffer */
> > -     addr = kmap_atomic(pages[0]);
> > +     addr = kmap_local_page(pages[0]);
> >       memcpy(buf, addr + off, sizes[0]);
> > -     kunmap_atomic(addr);
> > -     addr = kmap_atomic(pages[1]);
> > +     kunmap_local(addr);
> > +     addr = kmap_local_page(pages[1]);
> >       memcpy(buf + sizes[0], addr, sizes[1]);
> > -     kunmap_atomic(addr);
> > +     kunmap_local(addr);
>
> This looks like memcpy_from_page().
>
Yes, I checked and both the above memcpy can be replaced like this:
memcpy_from_page(buf, pages[0], off, sizes[0]);
memcpy_from_page(buf + sizes[0], pages[1], 0, sizes[1]);

> >       /* copy per-cpu buffer to object */
> > -     addr = kmap_atomic(pages[0]);
> > +     addr = kmap_local_page(pages[0]);
> >       memcpy(addr + off, buf, sizes[0]);
> > -     kunmap_atomic(addr);
> > -     addr = kmap_atomic(pages[1]);
> > +     kunmap_local(addr);
> > +     addr = kmap_local_page(pages[1]);
> >       memcpy(addr, buf + sizes[0], sizes[1]);
> > -     kunmap_atomic(addr);
> > +     kunmap_local(addr);
>
> memcpy_from_page()?
>
Same here, but I think this is memcpy_to_page().
I replaced it like this:
memcpy_to_page(page[0], off, buf, sizes[0]);
memcpy_to_page(page[1], 0, buf + sizes[0], sizes[1]);

> > @@ -1798,14 +1800,14 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
> >       migrate_write_lock(zspage);
> >
> >       offset = get_first_obj_offset(page);
> > -     s_addr = kmap_atomic(page);
> > +     s_addr = kmap_local_page(page);
> >
> >       /*
> >        * Here, any user cannot access all objects in the zspage so let's move.
> >        */
> > -     d_addr = kmap_atomic(newpage);
> > +     d_addr = kmap_local_page(newpage);
> >       copy_page(d_addr, s_addr);
> > -     kunmap_atomic(d_addr);
> > +     kunmap_local(d_addr);
>
> copy_highpage()?
>
This looks tricky. It does not look to be a straight-forward
replacement with copy_highpage.
There is a loop in-between which cannot be replaced I think.
I am checking more, but I need some help on this.

>
> Maybe check the other uses, see if there are appropriate helpers for
> them too.
>
Yes sure I am checking more.
Will share the changes with V2 in the new patchset.

> Also, what testing have you done of this patch?
My test setup is as follows:
Enabled: ZRAM, HIGHMEM in Kernel and compiled for arm32.
Used, qemu, arm32, 1GB RAM, ZRAM (128MB) to boot the device.
Using test program, filled zram area, then run stress-ng utility to
simulate memory pressure.
OOM occurred, freed some space, and again triggered allocation.
-------------
              total        used        free      shared  buff/cache   available
Mem:           1001         988           5           0           7           4
Swap:           127         127           0
Total:         1129        1116           5
Node 0, zone   Normal      2      1      9     15     11      2      1
     0      1      1      0
Node 0, zone  HighMem      0      2      5      6      0      0      0
     0      0      0      0

Thanks