| Message ID | 20231122182419.30633-6-fancer.lancer@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: mm: Fix some memory-related issues | expand |
Hi Serge, 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@gmail.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? > --- > 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 >
Hi Mike On Thu, Nov 23, 2023 at 12:18:54PM +0200, Mike Rapoport wrote: > Hi Serge, > > 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@gmail.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. 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). -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. >
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@gmail.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. > 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. 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. Now, if that hole is large you are wasting memory for unused memory map and it maybe worth considering using SPARSEMEM. > -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. > >
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@gmail.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.
On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > 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@gmail.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..( Do you mind posting your physical memory layout? If I understand correctly, you have a hole in your RAM and there is MMIO region somewhere in that hole. With FLATMEM the memory map exists for that hole and hence pfn_valid() returns 1 for the MMIO range as well. That makes __update_cache() to check folio state and that check would fail if the memory map contained garbage. But since the hole in the memory map is initialized with init_unavailable_range() you get a valid struct page/struct folio and everything is fine. With that, the init_unavailable_range() docs need not mention IO space at all, they should mention holes within FLATMEM memory map. As for SPARSEMEM, if the hole does not belong to any section, pfn_valid() will be false for it and __update_cache() won't try to access memory map. > > > 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.
On Tue, Nov 28, 2023 at 09:13:39AM +0200, Mike Rapoport wrote: > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > 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@gmail.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..( > > Do you mind posting your physical memory layout? I actually already did in response to the last part of your previous message. You must have missed it. Here is the copy of the message: > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > On Fri, Nov 24, 2023 at 10:19:00AM +0200, Mike Rapoport wrote: > > ... > > > > > > 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. > ... Could you also answer to my question above regarding using the sparsemem instead on my hw memory layout? > > If I understand correctly, you have a hole in your RAM and there is MMIO > region somewhere in that hole. Absolutely right. Please see my messages above. > With FLATMEM the memory map exists for that > hole and hence pfn_valid() returns 1 for the MMIO range as well. That makes > __update_cache() to check folio state and that check would fail if the memory > map contained garbage. But since the hole in the memory map is initialized > with init_unavailable_range() you get a valid struct page/struct folio and > everything is fine. Right. That's what currently happens on MIPS32 and that's what I had to fix in the framework of this series by the next patch: Link: https://lore.kernel.org/linux-mips/20231122182419.30633-4-fancer.lancer@gmail.com/ flatmem version of the pfn_valid() method has been broken due to max_mapnr being uninitialized before mem_init() is called. So init_unavailable_range() didn't initialize the pages on the early bootup stage. Thus afterwards, when max_mapnr has finally got a valid value any attempts to call the __update_cache() method on the MMIO memory hole caused the unaligned access crash. > > With that, the init_unavailable_range() docs need not mention IO space at > all, they should mention holes within FLATMEM memory map. Ok. I'll resend the patch with mentioning flatmem holes instead of mentioning the IO-spaces. > > As for SPARSEMEM, if the hole does not belong to any section, pfn_valid() > will be false for it and __update_cache() won't try to access memory map. Ah, I see. In case of the SPARSEMEM config an another version of pfn_valid() will be called. It's defined in the include/linux/mmzone.h header file. Right? If so then no problem there indeed. -Serge(y) > > > > > 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. > > -- > Sincerely yours, > Mike.
On Tue, Nov 28, 2023 at 01:51:32PM +0300, Serge Semin wrote: > On Tue, Nov 28, 2023 at 09:13:39AM +0200, Mike Rapoport wrote: > > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > > Do you mind posting your physical memory layout? > > I actually already did in response to the last part of your previous > message. You must have missed it. Here is the copy of the message: Sorry, for some reason I didn't scroll down your previous mail :) > > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > > On Fri, Nov 24, 2023 at 10:19:00AM +0200, Mike Rapoport wrote: > > > ... > > > > > > > > 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. > > Could you also answer to my question above regarding using the > sparsemem instead on my hw memory layout? Currently MIPS defines section size to 256MB, so with your memory layout with SPARSMEM there will be two sections of 256MB, at 0 and at 512MB, so you'll save memory map for 256M which is roughly 1M with 16k pages. It's possible With SPARSEMEM the pfn_to_page() and page_to_pfn() are a bit longer in terms of assembly instructions, but I really doubt you'll notice any performance difference in real world applications. > > With FLATMEM the memory map exists for that > > hole and hence pfn_valid() returns 1 for the MMIO range as well. That makes > > __update_cache() to check folio state and that check would fail if the memory > > map contained garbage. But since the hole in the memory map is initialized > > with init_unavailable_range() you get a valid struct page/struct folio and > > everything is fine. > > Right. That's what currently happens on MIPS32 and that's what I had > to fix in the framework of this series by the next patch: > Link: https://lore.kernel.org/linux-mips/20231122182419.30633-4-fancer.lancer@gmail.com/ > flatmem version of the pfn_valid() method has been broken due to > max_mapnr being uninitialized before mem_init() is called. So > init_unavailable_range() didn't initialize the pages on the early > bootup stage. Thus afterwards, when max_mapnr has finally got a valid > value any attempts to call the __update_cache() method on the MMIO > memory hole caused the unaligned access crash. The fix for max_mapnr makes pfn_valid()==1 for the entire memory map and this fixes up the struct pages in the hole. > > > > With that, the init_unavailable_range() docs need not mention IO space at > > all, they should mention holes within FLATMEM memory map. > > Ok. I'll resend the patch with mentioning flatmem holes instead of > mentioning the IO-spaces. > > > > > As for SPARSEMEM, if the hole does not belong to any section, pfn_valid() > > will be false for it and __update_cache() won't try to access memory map. > > Ah, I see. In case of the SPARSEMEM config an another version of > pfn_valid() will be called. It's defined in the include/linux/mmzone.h > header file. Right? If so then no problem there indeed. Yes, SPARSMEM uses pfn_valid() defined in include/linux/mmzone.h > -Serge(y)
On Wed, Nov 29, 2023 at 08:14:00AM +0200, Mike Rapoport wrote: > On Tue, Nov 28, 2023 at 01:51:32PM +0300, Serge Semin wrote: > > On Tue, Nov 28, 2023 at 09:13:39AM +0200, Mike Rapoport wrote: > > > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > > > > Do you mind posting your physical memory layout? > > > > I actually already did in response to the last part of your previous > > message. You must have missed it. Here is the copy of the message: > > Sorry, for some reason I didn't scroll down your previous mail :) > > > > On Fri, Nov 24, 2023 at 02:18:44PM +0300, Serge Semin wrote: > > > > On Fri, Nov 24, 2023 at 10:19:00AM +0200, Mike Rapoport wrote: > > > > ... > > > > > > > > > > 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. > > > > Could you also answer to my question above regarding using the > > sparsemem instead on my hw memory layout? > > Currently MIPS defines section size to 256MB, so with your memory layout > with SPARSMEM there will be two sections of 256MB, at 0 and at 512MB, so > you'll save memory map for 256M which is roughly 1M with 16k pages. > > It's possible > > With SPARSEMEM the pfn_to_page() and page_to_pfn() are a bit longer in > terms of assembly instructions, but I really doubt you'll notice any > performance difference in real world applications. Ok. Thank you very much for the comprehensive response. I'll give a good thought towards moving our platform to the sparse memory config. Most likely it will be done together with reducing SECTION_SIZE_BITS to 128MB in order to save a few more low-memory space. This will be mostly useful it XPA is enabled and 8GB memory is available. Such case requires a lot of low-memory for mapping, which is of just 128MB in our device. -Serge(y) > > > > With FLATMEM the memory map exists for that > > > hole and hence pfn_valid() returns 1 for the MMIO range as well. That makes > > > __update_cache() to check folio state and that check would fail if the memory > > > map contained garbage. But since the hole in the memory map is initialized > > > with init_unavailable_range() you get a valid struct page/struct folio and > > > everything is fine. > > > > Right. That's what currently happens on MIPS32 and that's what I had > > to fix in the framework of this series by the next patch: > > Link: https://lore.kernel.org/linux-mips/20231122182419.30633-4-fancer.lancer@gmail.com/ > > flatmem version of the pfn_valid() method has been broken due to > > max_mapnr being uninitialized before mem_init() is called. So > > init_unavailable_range() didn't initialize the pages on the early > > bootup stage. Thus afterwards, when max_mapnr has finally got a valid > > value any attempts to call the __update_cache() method on the MMIO > > memory hole caused the unaligned access crash. > > The fix for max_mapnr makes pfn_valid()==1 for the entire memory map and > this fixes up the struct pages in the hole. > > > > > > > With that, the init_unavailable_range() docs need not mention IO space at > > > all, they should mention holes within FLATMEM memory map. > > > > Ok. I'll resend the patch with mentioning flatmem holes instead of > > mentioning the IO-spaces. > > > > > > > > As for SPARSEMEM, if the hole does not belong to any section, pfn_valid() > > > will be false for it and __update_cache() won't try to access memory map. > > > > Ah, I see. In case of the SPARSEMEM config an another version of > > pfn_valid() will be called. It's defined in the include/linux/mmzone.h > > header file. Right? If so then no problem there indeed. > > Yes, SPARSMEM uses pfn_valid() defined in include/linux/mmzone.h > > > -Serge(y) > > -- > Sincerely yours, > Mike.
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 *
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@gmail.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. --- mm/mm_init.c | 1 + 1 file changed, 1 insertion(+)