Date:   Fri, 24 Nov 2023 14:18:44 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Mike Rapoport <rppt@...nel.org>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Matthew Wilcox <willy@...radead.org>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Arnd Bergmann <arnd@...db.de>,
        Aleksandar Rikalo <aleksandar.rikalo@...mia.com>,
        Aleksandar Rikalo <arikalo@...il.com>,
        Dragan Mladjenovic <dragan.mladjenovic@...mia.com>,
        Chao-ying Fu <cfu@...ecomp.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Yinglu Yang <yangyinglu@...ngson.cn>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Marc Zyngier <maz@...nel.org>, linux-mips@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/7] mm/mm_init.c: Extend init unavailable range doc info

On Fri, Nov 24, 2023 at 10:19:00AM +0200, Mike Rapoport wrote:
> On Thu, Nov 23, 2023 at 01:42:39PM +0300, Serge Semin wrote:
> > On Thu, Nov 23, 2023 at 12:18:54PM +0200, Mike Rapoport wrote:
> > > On Wed, Nov 22, 2023 at 09:24:03PM +0300, Serge Semin wrote:
> > > > Besides of the already described reasons the pages backended memory holes
> > > > might be persistent due to having memory mapped IO spaces behind those
> > > > ranges in the framework of flatmem kernel config. Add such note to the
> > > > init_unavailable_range() method kdoc in order to point out to one more
> > > > reason of having the function executed for such regions.
> > > > 
> > > > Signed-off-by: Serge Semin <fancer.lancer@...il.com>
> > > > 
> > > > ---
> > > > 
> > > > Please let me know if the IO-space pages must be initialized somehow
> > > > differently rather relying on free_area_init() executing the
> > > > init_unavailable_range() method.
> > > 
> > 
> > > Maybe I'm missing something, but why do you need struct pages in the
> > > IO space?
> > 
> > In my case at the very least that's due to having a SRAM device
> > available in the middle of the MMIO-space. The region is getting
> > mapped using the ioremap_wc() method (Uncached Write-Combine CA),
> > which eventually is converted to calling get_vm_area() and
> > ioremap_page_range() (see ioremap_prot() function on MIPS), which in
> > its turn use the page structs for mapping. Another similar case is
> > using ioremap_wc() in the PCIe outbound ATU space mapping of
> > the graphic/video cards framebuffers.
> 
> ioremap_page_range() does not need struct pages, but rather physical
> addresses.

Unless I miss something or MIPS32 is somehow special/wrong in that
matter, but from my just got experience it actually does at least in
the framework of the __update_cache() implementation which is called
in the set_ptes() method (former set_pte_at()), which in its turn
is eventually invoked by vmap_range_noflush() and finally by
ioremap_page_range(). See the patch
[PATCH 3/7] mips: Fix max_mapnr being uninitialized on early stages
Link: https://lore.kernel.org/linux-mips/20231122182419.30633-4-fancer.lancer@gmail.com/
of this series and the stack-trace of the bug fixed by that patch.

Is it wrong that on MIPS32 ioremap_page_range() eventually relies on
the page structs? It has been like that for, I don't know, long time.
If so then the sparse memory config might be broken on MIPS32..(

>  
> > In general having the pages array defined for the IO-memory is
> > required for mapping the IO-space other than just uncached (my sram
> > case for example) or, for instance, with special access attribute for
> > the user-space (if I am not missing something in a way VM works in
> > that case).
> 

> No, struct pages are not required to map IO space. If you need to map MMIO
> to userspace there's remap_pfn_range() for that.

Is this correct for both flat and sparse memory config? In anyway
please see my comment above about the problem I recently got.

> 
> My guess is that your system has a hole in the physical memory mappings and
> with FLATMEM that hole will have essentially unused struct pages, which are
> initialized by init_unavailable_range().  But from mm perspective this is
> still a hole even though there's some MMIO ranges in that hole.

Absolutely right. Here is the physical memory layout in my system.
0     - 128MB: RAM
128MB - 512MB: Memory mapped IO
512MB - 768MB..8.256GB: RAM

> 
> Now, if that hole is large you are wasting memory for unused memory map and
> it maybe worth considering using SPARSEMEM.

Do you think it's worth to move to the sparse memory configuration in
order to save the 384MB of mapping with the 16K page model? AFAIU flat
memory config is more performant. Performance is critical on the most
of the SoC applications especially when using the 10G ethernet or
the high-speed PCIe devices.

-Serge(y)

>  
> > -Serge(y)
> > 
> > > 
> > > > ---
> > > >  mm/mm_init.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > > > index 077bfe393b5e..3fa33e2d32ba 100644
> > > > --- a/mm/mm_init.c
> > > > +++ b/mm/mm_init.c
> > > > @@ -796,6 +796,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
> > > >   * - physical memory bank size is not necessarily the exact multiple of the
> > > >   *   arbitrary section size
> > > >   * - early reserved memory may not be listed in memblock.memory
> > > > + * - memory mapped IO space
> > > >   * - memory layouts defined with memmap= kernel parameter may not align
> > > >   *   nicely with memmap sections
> > > >   *
> > > > -- 
> > > > 2.42.1
> > > > 
> > > 
> > > -- 
> > > Sincerely yours,
> > > Mike.
> > > 
> 
> -- 
> Sincerely yours,
> Mike.

Powered by blists - more mailing lists