Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
2013/7/20 Andrew Morton : > On Fri, 19 Jul 2013 07:47:02 +0800 Jerry wrote: > >> 2013/7/19 Andrew Morton : >> > On Fri, 19 Jul 2013 00:56:12 +0800 Jerry wrote: >> > >> >> When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The >> >> calculating here will generate an unexpected result. In addition, if >> >> PAGE_SHIFT > 20, The memory size represented by numentries was already >> >> integral multiple of 1MB. >> >> >> > >> > If you tell me that you have a machine which has PAGE_SIZE=2MB and this >> > was the only problem which prevented Linux from running on that machine >> > then I'll apply the patch ;) >> > >> >> Hi Morton: >> I just "grep -rn "#define\s\+PAGE_SHIFT" arch/", and find the >> PAGE_SHIFT in some architecture is very big. >> such as the following in "arch/hexagon/include/asm/page.h" >> >> #ifdef CONFIG_PAGE_SIZE_256KB >> #define PAGE_SHIFT 18 >> #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_256KB >> #endif >> >> #ifdef CONFIG_PAGE_SIZE_1MB >> #define PAGE_SHIFT 20 >> #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB >> #endif >> . > > Good heavens. > >> In my patch, I think compiler would optimize "if (20 > PAGE_SIZE)", it >> won't generate any machine instruction. Just a guarantee. > > Well the existing code is a bit silly looking. Why can't we just do > > /* round applicable memory size up to nearest megabyte */ > if (PAGE_SHIFT < 20) > numentries = round_up(nr_kernel_pages, (1 << 20)/PAGE_SIZE); > > or similar? Great. I have adjusted these code lines, and sent the latest one. -- I love linux!!! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
On Fri, 19 Jul 2013 07:47:02 +0800 Jerry wrote: > 2013/7/19 Andrew Morton : > > On Fri, 19 Jul 2013 00:56:12 +0800 Jerry wrote: > > > >> When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The > >> calculating here will generate an unexpected result. In addition, if > >> PAGE_SHIFT > 20, The memory size represented by numentries was already > >> integral multiple of 1MB. > >> > > > > If you tell me that you have a machine which has PAGE_SIZE=2MB and this > > was the only problem which prevented Linux from running on that machine > > then I'll apply the patch ;) > > > > Hi Morton: > I just "grep -rn "#define\s\+PAGE_SHIFT" arch/", and find the > PAGE_SHIFT in some architecture is very big. > such as the following in "arch/hexagon/include/asm/page.h" > > #ifdef CONFIG_PAGE_SIZE_256KB > #define PAGE_SHIFT 18 > #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_256KB > #endif > > #ifdef CONFIG_PAGE_SIZE_1MB > #define PAGE_SHIFT 20 > #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB > #endif > . Good heavens. > In my patch, I think compiler would optimize "if (20 > PAGE_SIZE)", it > won't generate any machine instruction. Just a guarantee. Well the existing code is a bit silly looking. Why can't we just do /* round applicable memory size up to nearest megabyte */ if (PAGE_SHIFT < 20) numentries = round_up(nr_kernel_pages, (1 << 20)/PAGE_SIZE); or similar? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
2013/7/19 Andrew Morton : > On Fri, 19 Jul 2013 00:56:12 +0800 Jerry wrote: > >> When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The >> calculating here will generate an unexpected result. In addition, if >> PAGE_SHIFT > 20, The memory size represented by numentries was already >> integral multiple of 1MB. >> > > If you tell me that you have a machine which has PAGE_SIZE=2MB and this > was the only problem which prevented Linux from running on that machine > then I'll apply the patch ;) > Hi Morton: I just "grep -rn "#define\s\+PAGE_SHIFT" arch/", and find the PAGE_SHIFT in some architecture is very big. such as the following in "arch/hexagon/include/asm/page.h" #ifdef CONFIG_PAGE_SIZE_256KB #define PAGE_SHIFT 18 #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_256KB #endif #ifdef CONFIG_PAGE_SIZE_1MB #define PAGE_SHIFT 20 #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB #endif . Maybe the day of "A 2MB page" is not far. :-) I know it is just a latent issue. Even if it won't generate a error when PAGE_SIZE == 20, the calculating here is not necessary. In my mind, compiler would optimize the calculating at that situation. But it is a little tricky. In my patch, I think compiler would optimize "if (20 > PAGE_SIZE)", it won't generate any machine instruction. Just a guarantee. -- I love linux!!! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
On Fri, 19 Jul 2013 00:56:12 +0800 Jerry wrote: > When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The > calculating here will generate an unexpected result. In addition, if > PAGE_SHIFT > 20, The memory size represented by numentries was already > integral multiple of 1MB. > If you tell me that you have a machine which has PAGE_SIZE=2MB and this was the only problem which prevented Linux from running on that machine then I'll apply the patch ;) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
Jerry> When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The
Jerry> calculating here will generate an unexpected result. In addition, if
Jerry> PAGE_SHIFT > 20, The memory size represented by numentries was already
Jerry> integral multiple of 1MB.
Why this magic number of 20? Please explain it better and replace it
was a #define that means something here.
Jerry> Signed-off-by: Jerry
Jerry> ---
Jerry> mm/page_alloc.c | 8 +---
Jerry> 1 file changed, 5 insertions(+), 3 deletions(-)
Jerry> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
Jerry> index b100255..cd41797 100644
Jerry> --- a/mm/page_alloc.c
Jerry> +++ b/mm/page_alloc.c
Jerry> @@ -5745,9 +5745,11 @@ void *__init alloc_large_system_hash(const char
*tablename,
Jerry> if (!numentries) {
Jerry> /* round applicable memory size up to nearest megabyte
*/
Jerry> numentries = nr_kernel_pages;
Jerry> -numentries += (1UL << (20 - PAGE_SHIFT)) - 1;
Jerry> -numentries >>= 20 - PAGE_SHIFT;
Jerry> -numentries <<= 20 - PAGE_SHIFT;
Jerry> +if (20 > PAGE_SHIFT) {
Jerry> +numentries += (1UL << (20 - PAGE_SHIFT)) - 1;
Jerry> +numentries >>= 20 - PAGE_SHIFT;
Jerry> +numentries <<= 20 - PAGE_SHIFT;
Jerry> +}
Jerry> /* limit to 1 bucket per 2^scale bytes of low memory */
Jerry> if (scale > PAGE_SHIFT)
Jerry> --
Jerry> 1.8.1.5
Jerry> --
Jerry> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
in
Jerry> the body of a message to majord...@vger.kernel.org
Jerry> More majordomo info at http://vger.kernel.org/majordomo-info.html
Jerry> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: negative left shift count when PAGE_SHIFT > 20
> Jerry> When PAGE_SHIFT > 20, the result of "20 - PAGE_SHIFT" is negative. The
> Jerry> calculating here will generate an unexpected result. In addition, if
> Jerry> PAGE_SHIFT > 20, The memory size represented by numentries was already
> Jerry> integral multiple of 1MB.
>
> Why this magic number of 20? Please explain it better and replace it
> was a #define that means something here.
Because 2^20 = 1MB.
The intention of previous code is "/* round applicable memory size up
to nearest megabyte */".
>
> Jerry> Signed-off-by: Jerry
> Jerry> ---
> Jerry> mm/page_alloc.c | 8 +---
> Jerry> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> Jerry> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> Jerry> index b100255..cd41797 100644
> Jerry> --- a/mm/page_alloc.c
> Jerry> +++ b/mm/page_alloc.c
> Jerry> @@ -5745,9 +5745,11 @@ void *__init alloc_large_system_hash(const char
> *tablename,
> Jerry> if (!numentries) {
> Jerry> /* round applicable memory size up to nearest
> megabyte */
> Jerry> numentries = nr_kernel_pages;
> Jerry> -numentries += (1UL << (20 - PAGE_SHIFT)) - 1;
> Jerry> -numentries >>= 20 - PAGE_SHIFT;
> Jerry> -numentries <<= 20 - PAGE_SHIFT;
> Jerry> +if (20 > PAGE_SHIFT) {
> Jerry> +numentries += (1UL << (20 - PAGE_SHIFT)) - 1;
> Jerry> +numentries >>= 20 - PAGE_SHIFT;
> Jerry> +numentries <<= 20 - PAGE_SHIFT;
> Jerry> +}
>
> Jerry> /* limit to 1 bucket per 2^scale bytes of low memory
> */
> Jerry> if (scale > PAGE_SHIFT)
> Jerry> --
> Jerry> 1.8.1.5
>
> Jerry> --
> Jerry> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> Jerry> the body of a message to majord...@vger.kernel.org
> Jerry> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Jerry> Please read the FAQ at http://www.tux.org/lkml/
--
I love linux!!!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
