On LoongArch, the physical address space ranging from 0 to 0xfffffff is
always memory, which is in the low half of the memsection range from 0 to
0x1fffffff with 512M memsection size, and the high half will be a hole with
invalid memory pages.
This situation may cause some issues. For example, the range of 0x10000000
to 0x1fffffff is io registers, which will be considered as valid memory range
since which is in the memsection of range of 0 to 0x1fffffff. During S3
sleep and resume, these io registers will be saved and restored as valid memory
page (pfn_valid() of common version considers that all pages in a memsection
are valid), which will cause exception, especially when restoring during resume.
We can use 256M size for memsection of LoongArch, or use the way as ARM64 to
walk through all memory memblock to check if a mem pfn is valid which maybe
lower performance. For simplicity, this patch just use the former way.
Signed-off-by: Jianmin Lv <[email protected]>
diff --git a/arch/loongarch/include/asm/sparsemem.h b/arch/loongarch/include/asm/sparsemem.h
index 3d18cdf1b069..05903b40a625 100644
--- a/arch/loongarch/include/asm/sparsemem.h
+++ b/arch/loongarch/include/asm/sparsemem.h
@@ -8,7 +8,7 @@
* SECTION_SIZE_BITS 2^N: how big each section will be
* MAX_PHYSMEM_BITS 2^N: how much memory we can have in that space
*/
-#define SECTION_SIZE_BITS 29 /* 2^29 = Largest Huge Page Size */
+#define SECTION_SIZE_BITS 28
#define MAX_PHYSMEM_BITS 48
#endif /* CONFIG_SPARSEMEM */
--
2.31.1
On 2022/10/26 15:56, Jianmin Lv wrote:
> On LoongArch, the physical address space ranging from 0 to 0xfffffff is
> always memory, which is in the low half of the memsection range from 0 to
> 0x1fffffff with 512M memsection size, and the high half will be a hole with
> invalid memory pages.
The description is incorrect. For systems with less than 512MiB of
memory for example, I believe not every address from 0x0 to 0x0fff_ffff
is valid; and regarding the latter part of the sentence, what you mean
by "invalid memory pages"...
>
> This situation may cause some issues. For example, the range of 0x10000000
> to 0x1fffffff is io registers, which will be considered as valid memory range
> since which is in the memsection of range of 0 to 0x1fffffff. During S3
... turns out to be totally valid, only of the I/O kind. (This might be
a case of Chinglish that is actually conveying the incorrect meaning to
unaware readers.)
> sleep and resume, these io registers will be saved and restored as valid memory
> page (pfn_valid() of common version considers that all pages in a memsection
> are valid), which will cause exception, especially when restoring during resume.
>
> We can use 256M size for memsection of LoongArch, or use the way as ARM64 to
> walk through all memory memblock to check if a mem pfn is valid which maybe
> lower performance. For simplicity, this patch just use the former way.
And the rest of the commit message is, unfortunately, a bit too verbose
and hard to understand in general. I have to look at the actual change
(luckily, a one-liner in this case) to confirm my understanding.
I think your intent is just to *avoid stepping into IO memory region
during suspend/resume by reducing the section size order by one*. Try
reducing the verbosity of the commit message in v2? I can't proofread
and edit every commit due to limited time, so you have to practice and
improve your writing skills after all. I'll review that piece of text
afterwards. :)
>
> Signed-off-by: Jianmin Lv <[email protected]>
>
> diff --git a/arch/loongarch/include/asm/sparsemem.h b/arch/loongarch/include/asm/sparsemem.h
> index 3d18cdf1b069..05903b40a625 100644
> --- a/arch/loongarch/include/asm/sparsemem.h
> +++ b/arch/loongarch/include/asm/sparsemem.h
> @@ -8,7 +8,7 @@
> * SECTION_SIZE_BITS 2^N: how big each section will be
> * MAX_PHYSMEM_BITS 2^N: how much memory we can have in that space
> */
> -#define SECTION_SIZE_BITS 29 /* 2^29 = Largest Huge Page Size */
> +#define SECTION_SIZE_BITS 28
> #define MAX_PHYSMEM_BITS 48
>
> #endif /* CONFIG_SPARSEMEM */
The change is trivial indeed but I'm not immediately giving the R-b.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
On 2022/10/26 下午5:21, WANG Xuerui wrote:
> On 2022/10/26 15:56, Jianmin Lv wrote:
>> On LoongArch, the physical address space ranging from 0 to 0xfffffff is
>> always memory, which is in the low half of the memsection range from 0 to
>> 0x1fffffff with 512M memsection size, and the high half will be a hole
>> with
>> invalid memory pages.
>
> The description is incorrect. For systems with less than 512MiB of
> memory for example, I believe not every address from 0x0 to 0x0fff_ffff
> is valid;
As far as I know, almost all designs use 0x0 to 0x0fff_ffff as memory
even with 512M because it's difficult to set cpu to support a
combination with memory and non-memory for 0x0 to 0x0fff_ffff.
> and regarding the latter part of the sentence, what you mean
> by "invalid memory pages"...
>
If with 512M memsection size, for the memsection from 0 to 0x1fffffff,
low half (0 to 0xfffffff) is valid memory, and high half(0x10000000 to
0x1fffffff ) is I/O registers (of chipsets and cpu), so high half is not
memory, I think it's better to change *with invalid memory pages* to
*which is not memory.*
>>
>> This situation may cause some issues. For example, the range of
>> 0x10000000
>> to 0x1fffffff is io registers, which will be considered as valid
>> memory range
>> since which is in the memsection of range of 0 to 0x1fffffff. During S3
>
> ... turns out to be totally valid, only of the I/O kind. (This might be
> a case of Chinglish that is actually conveying the incorrect meaning to
> unaware readers.)
>
The whole range of the 512M memsection will be totally valid by
pfn_valid(), and let me try reduce the example description. :)
>> sleep and resume, these io registers will be saved and restored as
>> valid memory
>> page (pfn_valid() of common version considers that all pages in a
>> memsection
>> are valid), which will cause exception, especially when restoring
>> during resume.
>>
>> We can use 256M size for memsection of LoongArch, or use the way as
>> ARM64 to
>> walk through all memory memblock to check if a mem pfn is valid which
>> maybe
>> lower performance. For simplicity, this patch just use the former way.
>
> And the rest of the commit message is, unfortunately, a bit too verbose
> and hard to understand in general. I have to look at the actual change
> (luckily, a one-liner in this case) to confirm my understanding.
>
> I think your intent is just to *avoid stepping into IO memory region
> during suspend/resume by reducing the section size order by one*.
Yes.
> Try reducing the verbosity of the commit message in v2?
Ok, let me try. :)
> I can't proofread and edit every commit due to limited time, so you have to practice and
> improve your writing skills after all. I'll review that piece of text
> afterwards. :)
>
Ok, thanks.
>>
>> Signed-off-by: Jianmin Lv <[email protected]>
>>
>> diff --git a/arch/loongarch/include/asm/sparsemem.h
>> b/arch/loongarch/include/asm/sparsemem.h
>> index 3d18cdf1b069..05903b40a625 100644
>> --- a/arch/loongarch/include/asm/sparsemem.h
>> +++ b/arch/loongarch/include/asm/sparsemem.h
>> @@ -8,7 +8,7 @@
>> * SECTION_SIZE_BITS 2^N: how big each section will be
>> * MAX_PHYSMEM_BITS 2^N: how much memory we can have in that
>> space
>> */
>> -#define SECTION_SIZE_BITS 29 /* 2^29 = Largest Huge Page Size */
>> +#define SECTION_SIZE_BITS 28
>> #define MAX_PHYSMEM_BITS 48
>> #endif /* CONFIG_SPARSEMEM */
>
> The change is trivial indeed but I'm not immediately giving the R-b.
>
Hi Jianmin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc2 next-20221026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jianmin-Lv/LoongArch-Fix-memsection-size/20221026-155924
patch link: https://lore.kernel.org/r/20221026075638.9396-1-lvjianmin%40loongson.cn
patch subject: [PATCH] LoongArch: Fix memsection size
config: loongarch-defconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/6f7453a9afb01ac06032833503f47f335a2623f9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jianmin-Lv/LoongArch-Fix-memsection-size/20221026-155924
git checkout 6f7453a9afb01ac06032833503f47f335a2623f9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> mm/memory.c:95:2: warning: #warning Unfortunate NUMA and NUMA Balancing config, growing page-frame for last_cpupid. [-Wcpp]
95 | #warning Unfortunate NUMA and NUMA Balancing config, growing page-frame for last_cpupid.
| ^~~~~~~
vim +95 mm/memory.c
42b7772812d15b Jan Beulich 2008-07-23 93
af27d9403f5b80 Arnd Bergmann 2018-02-16 94 #if defined(LAST_CPUPID_NOT_IN_PAGE_FLAGS) && !defined(CONFIG_COMPILE_TEST)
90572890d20252 Peter Zijlstra 2013-10-07 @95 #warning Unfortunate NUMA and NUMA Balancing config, growing page-frame for last_cpupid.
75980e97daccfc Peter Zijlstra 2013-02-22 96 #endif
75980e97daccfc Peter Zijlstra 2013-02-22 97
--
0-DAY CI Kernel Test Service
https://01.org/lkp