LoongArch: Fix memsection size

  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 <lvjianmin@loongson.cn>
  

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 <lvjianmin@loongson.cn>
> 
> 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.
  

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 <lvjianmin@loongson.cn>
>>
>> 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 <lkp@intel.com>

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