2021-05-19 17:55:25

by Xu, Yanfei

[permalink] [raw]
Subject: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform

The value of "vmalloc=" set in cmdline is always 8M more than the value
of "VmallocTotal" in meminfo. When use the "vmalloc=" parameter, user
expect to get the size what they input, and no need to consider the 8M
"hole" hided in codes. This commit make real vmalloc size equal to value
of "vmalloc=" in cmdline.

Also, the commit will reduce the size of vmalloc printed in boot message
by 8M when the size set in cmdline is irrational.

Signed-off-by: Yanfei Xu <[email protected]>
---
arch/arm/mm/mmu.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index c1e12aab67b8..287c5115af4d 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1133,19 +1133,20 @@ static int __init early_vmalloc(char *arg)
{
unsigned long vmalloc_reserve = memparse(arg, NULL);

- if (vmalloc_reserve < SZ_16M) {
- vmalloc_reserve = SZ_16M;
+ vmalloc_reserve = ALIGN_DOWN(vmalloc_reserve, SZ_8M);
+ if (vmalloc_reserve < SZ_8M) {
+ vmalloc_reserve = SZ_8M;
pr_warn("vmalloc area too small, limiting to %luMB\n",
vmalloc_reserve >> 20);
}

if (vmalloc_reserve > VMALLOC_END - (PAGE_OFFSET + SZ_32M)) {
- vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
pr_warn("vmalloc area is too big, limiting to %luMB\n",
vmalloc_reserve >> 20);
}

- vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve);
+ vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve - VMALLOC_OFFSET);
return 0;
}
early_param("vmalloc", early_vmalloc);
--
2.27.0



2021-05-19 17:56:44

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform

On Tue, May 18, 2021 at 07:12:54PM +0800, Yanfei Xu wrote:
> The value of "vmalloc=" set in cmdline is always 8M more than the value
> of "VmallocTotal" in meminfo. When use the "vmalloc=" parameter, user
> expect to get the size what they input, and no need to consider the 8M
> "hole" hided in codes. This commit make real vmalloc size equal to value
> of "vmalloc=" in cmdline.
>
> Also, the commit will reduce the size of vmalloc printed in boot message
> by 8M when the size set in cmdline is irrational.

Hi,

I think I'd like to do several cleanups with this:

1. change vmalloc_min to be an unsigned long.
2. exclude VMALLOC_OFFSET from vmalloc_min, moving it into
adjust_lowmem_bounds where vmalloc_min is used.
3. rename vmalloc_min to be vmalloc_start
4. enforce vmalloc_start to be a multiple of 2MiB
5. in early_vmalloc(), calculate vmalloc_max as:
VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET)
and use that to set the upper bound of vmalloc_reserve (which is
something your patch doesn't do, which I think is a bug.

Thoughts?

>
> Signed-off-by: Yanfei Xu <[email protected]>
> ---
> arch/arm/mm/mmu.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index c1e12aab67b8..287c5115af4d 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1133,19 +1133,20 @@ static int __init early_vmalloc(char *arg)
> {
> unsigned long vmalloc_reserve = memparse(arg, NULL);
>
> - if (vmalloc_reserve < SZ_16M) {
> - vmalloc_reserve = SZ_16M;
> + vmalloc_reserve = ALIGN_DOWN(vmalloc_reserve, SZ_8M);
> + if (vmalloc_reserve < SZ_8M) {
> + vmalloc_reserve = SZ_8M;
> pr_warn("vmalloc area too small, limiting to %luMB\n",
> vmalloc_reserve >> 20);
> }
>
> if (vmalloc_reserve > VMALLOC_END - (PAGE_OFFSET + SZ_32M)) {
> - vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
> + vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
> pr_warn("vmalloc area is too big, limiting to %luMB\n",
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve);
> + vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve - VMALLOC_OFFSET);
> return 0;
> }
> early_param("vmalloc", early_vmalloc);
> --
> 2.27.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-05-19 18:04:02

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform

On Tue, May 18, 2021 at 12:29:32PM +0100, Russell King (Oracle) wrote:
> On Tue, May 18, 2021 at 07:12:54PM +0800, Yanfei Xu wrote:
> > The value of "vmalloc=" set in cmdline is always 8M more than the value
> > of "VmallocTotal" in meminfo. When use the "vmalloc=" parameter, user
> > expect to get the size what they input, and no need to consider the 8M
> > "hole" hided in codes. This commit make real vmalloc size equal to value
> > of "vmalloc=" in cmdline.
> >
> > Also, the commit will reduce the size of vmalloc printed in boot message
> > by 8M when the size set in cmdline is irrational.
>
> Hi,
>
> I think I'd like to do several cleanups with this:
>
> 1. change vmalloc_min to be an unsigned long.
> 2. exclude VMALLOC_OFFSET from vmalloc_min, moving it into
> adjust_lowmem_bounds where vmalloc_min is used.
> 3. rename vmalloc_min to be vmalloc_start
> 4. enforce vmalloc_start to be a multiple of 2MiB
> 5. in early_vmalloc(), calculate vmalloc_max as:
> VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET)
> and use that to set the upper bound of vmalloc_reserve (which is
> something your patch doesn't do, which I think is a bug.
>
> Thoughts?

I've slightly modified the above idea, and will shortly follow up with
some patches to show the idea a bit better...

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-05-19 18:04:05

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH 1/4] ARM: change vmalloc_min to be unsigned long

vmalloc_min is currently a void pointer, but everywhere its used
contains a cast - either to a void pointer when setting or back to
an integer type when being used. Eliminate these casts by changing
its type to unsigned long.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index c06ebfbc48c4..206c345f063e 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,8 +1123,8 @@ void __init debug_ll_io_init(void)
}
#endif

-static void * __initdata vmalloc_min =
- (void *)(VMALLOC_END - (240 << 20) - VMALLOC_OFFSET);
+static unsigned long __initdata vmalloc_min =
+ VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1147,7 +1147,7 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve);
+ vmalloc_min = VMALLOC_END - vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1167,7 +1167,7 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)(uintptr_t)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
+ vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;

/*
* The first usable region must be PMD aligned. Mark its start
--
2.20.1


2021-05-19 18:04:38

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

Change the current vmalloc_min, which is supposed to be the lowest
address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
which does not include VMALLOC_OFFSET.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index d932c46a02e0..457203b41ceb 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,8 +1123,7 @@ void __init debug_ll_io_init(void)
}
#endif

-static unsigned long __initdata vmalloc_min =
- VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
+static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1142,14 +1141,14 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
if (vmalloc_reserve > vmalloc_max) {
vmalloc_reserve = vmalloc_max;
pr_warn("vmalloc area is too big, limiting to %luMB\n",
vmalloc_reserve >> 20);
}

- vmalloc_min = VMALLOC_END - vmalloc_reserve;
+ vmalloc_start = VMALLOC_END - vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
+ vmalloc_limit = (u64)vmalloc_start -
+ (PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);

/*
* The first usable region must be PMD aligned. Mark its start
--
2.20.1


2021-05-19 18:04:55

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH 2/4] ARM: use a temporary variable to hold maximum vmalloc size

We calculate the maximum size of the vmalloc space twice in
early_vmalloc(). Use a temporary variable to hold this value.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 206c345f063e..d932c46a02e0 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1134,6 +1134,7 @@ static unsigned long __initdata vmalloc_min =
static int __init early_vmalloc(char *arg)
{
unsigned long vmalloc_reserve = memparse(arg, NULL);
+ unsigned long vmalloc_max;

if (vmalloc_reserve < SZ_16M) {
vmalloc_reserve = SZ_16M;
@@ -1141,8 +1142,9 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- if (vmalloc_reserve > VMALLOC_END - (PAGE_OFFSET + SZ_32M)) {
- vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ if (vmalloc_reserve > vmalloc_max) {
+ vmalloc_reserve = vmalloc_max;
pr_warn("vmalloc area is too big, limiting to %luMB\n",
vmalloc_reserve >> 20);
}
--
2.20.1


2021-05-19 18:05:00

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH 4/4] ARM: change vmalloc_start to vmalloc_size

Rather than storing the start of vmalloc space, store the size, and
move the calculation into adjust_lowmem_limit(). We now have one single
place where this calculation takes place.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 457203b41ceb..2c74f1230cc7 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,7 +1123,7 @@ void __init debug_ll_io_init(void)
}
#endif

-static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
+static unsigned long __initdata vmalloc_size = 240 << 20;

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1148,7 +1148,7 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_start = VMALLOC_END - vmalloc_reserve;
+ vmalloc_size = vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1168,7 +1168,7 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)vmalloc_start -
+ vmalloc_limit = (u64)(VMALLOC_END - vmalloc_size) -
(PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);

/*
--
2.20.1


2021-05-19 18:40:45

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/4] ARM: change vmalloc_min to be unsigned long

On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
<[email protected]> wrote:

> vmalloc_min is currently a void pointer, but everywhere its used
> contains a cast - either to a void pointer when setting or back to
> an integer type when being used. Eliminate these casts by changing
> its type to unsigned long.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Looks good!
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-19 18:42:20

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Wed, May 19, 2021 at 12:32 AM Nicolas Pitre <[email protected]> wrote:
> On Tue, 18 May 2021, Nicolas Pitre wrote:
> > On Wed, 19 May 2021, Linus Walleij wrote:
> > > please fold in a comment
> > > with some explanation of that (240 << 20) thing,
> >
> > That's an alternative (and deprecated) way to write MB(240).
>
> And it seems that MB() isn't globally defined either. Oh well.

I suppose a comment saying "set vmalloc to be 240 MB" is fair enough.

(And I do feel silly for not realizing that...)

Yours,
Linus Walleij

2021-05-19 18:42:39

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Wed, 19 May 2021, Linus Walleij wrote:

> On Wed, May 19, 2021 at 12:32 AM Nicolas Pitre <[email protected]> wrote:
> > On Tue, 18 May 2021, Nicolas Pitre wrote:
> > > On Wed, 19 May 2021, Linus Walleij wrote:
> > > > please fold in a comment
> > > > with some explanation of that (240 << 20) thing,
> > >
> > > That's an alternative (and deprecated) way to write MB(240).
> >
> > And it seems that MB() isn't globally defined either. Oh well.
>
> I suppose a comment saying "set vmalloc to be 240 MB" is fair enough.

The comment was some 5 lines below (not ideal I know).


Nicolas

2021-05-19 18:42:49

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: use a temporary variable to hold maximum vmalloc size

On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
<[email protected]> wrote:

> We calculate the maximum size of the vmalloc space twice in
> early_vmalloc(). Use a temporary variable to hold this value.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-19 18:43:10

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
<[email protected]> wrote:

> Change the current vmalloc_min, which is supposed to be the lowest
> address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> which does not include VMALLOC_OFFSET.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

> +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);

When I first read this it took me some time to figure out what was
going on here, so if you have time, please fold in a comment
with some explanation of that (240 << 20) thing, in some blog
post I described it as "an interesting way to write 0x0f000000"
but I suppose commit 0536bdf33faf chose this way for a
specific reason? (Paging Nico if he can explain it.)

Yours,
Linus Walleij

2021-05-19 18:43:23

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 4/4] ARM: change vmalloc_start to vmalloc_size

On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
<[email protected]> wrote:

> Rather than storing the start of vmalloc space, store the size, and
> move the calculation into adjust_lowmem_limit(). We now have one single
> place where this calculation takes place.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

This is really nice.
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-19 18:43:28

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Wed, 19 May 2021, Linus Walleij wrote:

> On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
> <[email protected]> wrote:
>
> > Change the current vmalloc_min, which is supposed to be the lowest
> > address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> > which does not include VMALLOC_OFFSET.
> >
> > Signed-off-by: Russell King (Oracle) <[email protected]>
>
> Reviewed-by: Linus Walleij <[email protected]>
>
> > +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
>
> When I first read this it took me some time to figure out what was
> going on here, so if you have time, please fold in a comment
> with some explanation of that (240 << 20) thing, in some blog
> post I described it as "an interesting way to write 0x0f000000"
> but I suppose commit 0536bdf33faf chose this way for a
> specific reason? (Paging Nico if he can explain it.)

That's an alternative (and deprecated) way to write MB(240).


Nicolas

2021-05-19 19:12:12

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform



On 5/18/21 8:06 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tue, May 18, 2021 at 12:29:32PM +0100, Russell King (Oracle) wrote:
>> On Tue, May 18, 2021 at 07:12:54PM +0800, Yanfei Xu wrote:
>>> The value of "vmalloc=" set in cmdline is always 8M more than the value
>>> of "VmallocTotal" in meminfo. When use the "vmalloc=" parameter, user
>>> expect to get the size what they input, and no need to consider the 8M
>>> "hole" hided in codes. This commit make real vmalloc size equal to value
>>> of "vmalloc=" in cmdline.
>>>
>>> Also, the commit will reduce the size of vmalloc printed in boot message
>>> by 8M when the size set in cmdline is irrational.
>>
>> Hi,
>>
>> I think I'd like to do several cleanups with this:
>>
>> 1. change vmalloc_min to be an unsigned long.
>> 2. exclude VMALLOC_OFFSET from vmalloc_min, moving it into
>> adjust_lowmem_bounds where vmalloc_min is used.
>> 3. rename vmalloc_min to be vmalloc_start
>> 4. enforce vmalloc_start to be a multiple of 2MiB
>> 5. in early_vmalloc(), calculate vmalloc_max as:
>> VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET)
>> and use that to set the upper bound of vmalloc_reserve (which is
>> something your patch doesn't do, which I think is a bug.
>>
>> Thoughts?
>
> I've slightly modified the above idea, and will shortly follow up with
> some patches to show the idea a bit better...
>

Thanks for making it better!

Regards,
Yanfei
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
>

2021-05-19 19:12:15

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start



On 5/18/21 8:15 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Change the current vmalloc_min, which is supposed to be the lowest
> address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> which does not include VMALLOC_OFFSET.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>
> ---
> arch/arm/mm/mmu.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index d932c46a02e0..457203b41ceb 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1123,8 +1123,7 @@ void __init debug_ll_io_init(void)
> }
> #endif
>
> -static unsigned long __initdata vmalloc_min =
> - VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
> +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
>
> /*
> * vmalloc=size forces the vmalloc area to be exactly 'size'
> @@ -1142,14 +1141,14 @@ static int __init early_vmalloc(char *arg)
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
> + vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
> if (vmalloc_reserve > vmalloc_max) {
> vmalloc_reserve = vmalloc_max;
> pr_warn("vmalloc area is too big, limiting to %luMB\n",
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_min = VMALLOC_END - vmalloc_reserve;
> + vmalloc_start = VMALLOC_END - vmalloc_reserve;
> return 0;
> }
> early_param("vmalloc", early_vmalloc);
> @@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
> * and may itself be outside the valid range for which phys_addr_t
> * and therefore __pa() is defined.
> */
> - vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
> + vmalloc_limit = (u64)vmalloc_start -
> + (PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);
>
Here is bug, it should be

vmalloc_limit = (u64)vmalloc_start -
(PAGE_OFFSET + VMALLOC_OFFSET) + PHYS_OFFSET;

> /*
> * The first usable region must be PMD aligned. Mark its start
> --
> 2.20.1
>

2021-05-19 19:13:01

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start



On 5/18/21 8:15 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Change the current vmalloc_min, which is supposed to be the lowest
> address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> which does not include VMALLOC_OFFSET.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>
> ---
> arch/arm/mm/mmu.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index d932c46a02e0..457203b41ceb 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1123,8 +1123,7 @@ void __init debug_ll_io_init(void)
> }
> #endif
>
> -static unsigned long __initdata vmalloc_min =
> - VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
> +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
>
> /*
> * vmalloc=size forces the vmalloc area to be exactly 'size'
> @@ -1142,14 +1141,14 @@ static int __init early_vmalloc(char *arg)
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
> + vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
> if (vmalloc_reserve > vmalloc_max) {
> vmalloc_reserve = vmalloc_max;
> pr_warn("vmalloc area is too big, limiting to %luMB\n",
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_min = VMALLOC_END - vmalloc_reserve;
> + vmalloc_start = VMALLOC_END - vmalloc_reserve;
> return 0;
> }
> early_param("vmalloc", early_vmalloc);

The minimal vamlloc size should reduce 8MB, to be same with the original
vmalloc size.

@@ -1133,8 +1133,8 @@ static int __init early_vmalloc(char *arg)
unsigned long vmalloc_reserve = memparse(arg, NULL);
unsigned long vmalloc_max;

- if (vmalloc_reserve < SZ_16M) {
- vmalloc_reserve = SZ_16M;
+ if (vmalloc_reserve < SZ_8M) {
+ vmalloc_reserve = SZ_8M;
pr_warn("vmalloc area too small, limiting to %luMB\n",
vmalloc_reserve >> 20);
}

Another point, the current size of "vmalloc=" will be align up with 8MB,
should we align it down? The original is align down when we consider the
vmalloc_offest in vmalloc size. If yes, we could do it like

index eb6173315291..1fc2696fadd2 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1133,8 +1133,9 @@ static int __init early_vmalloc(char *arg)
unsigned long vmalloc_reserve = memparse(arg, NULL);
unsigned long vmalloc_max;

- if (vmalloc_reserve < SZ_16M) {
- vmalloc_reserve = SZ_16M;
+ vmalloc_reserve = ALIGN_DOWN(vmalloc_reserve, SZ_8M);
+ if (vmalloc_reserve < SZ_8M) {
+ vmalloc_reserve = SZ_8M;
pr_warn("vmalloc area too small, limiting to %luMB\n",
vmalloc_reserve >> 20);
}


Regards,
Yanfei


> @@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
> * and may itself be outside the valid range for which phys_addr_t
> * and therefore __pa() is defined.
> */
> - vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
> + vmalloc_limit = (u64)vmalloc_start -
> + (PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);
>
> /*
> * The first usable region must be PMD aligned. Mark its start
> --
> 2.20.1
>

2021-05-19 19:13:19

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform



On 5/19/21 12:39 PM, Xu, Yanfei wrote:
>
>
> On 5/18/21 8:06 PM, Russell King (Oracle) wrote:
>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>
>> On Tue, May 18, 2021 at 12:29:32PM +0100, Russell King (Oracle) wrote:
>>> On Tue, May 18, 2021 at 07:12:54PM +0800, Yanfei Xu wrote:
>>>> The value of "vmalloc=" set in cmdline is always 8M more than the value
>>>> of "VmallocTotal" in meminfo. When use the "vmalloc=" parameter, user
>>>> expect to get the size what they input, and no need to consider the 8M
>>>> "hole" hided in codes. This commit make real vmalloc size equal to
>>>> value
>>>> of "vmalloc=" in cmdline.
>>>>
>>>> Also, the commit will reduce the size of vmalloc printed in boot
>>>> message
>>>> by 8M when the size set in cmdline is irrational.
>>>
>>> Hi,
>>>
>>> I think I'd like to do several cleanups with this:
>>>
>>> 1. change vmalloc_min to be an unsigned long.
>>> 2. exclude VMALLOC_OFFSET from vmalloc_min, moving it into
>>>     adjust_lowmem_bounds where vmalloc_min is used.
>>> 3. rename vmalloc_min to be vmalloc_start
>>> 4. enforce vmalloc_start to be a multiple of 2MiB
>>> 5. in early_vmalloc(), calculate vmalloc_max as:
>>>        VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET)
>>>     and use that to set the upper bound of vmalloc_reserve (which is
>>>     something your patch doesn't do, which I think is a bug.
>>>
>>> Thoughts?
>>
>> I've slightly modified the above idea, and will shortly follow up with
>> some patches to show the idea a bit better...
>>
>
> Thanks for making it better!
>
Hi Russell,

I am not much familar with community contribution. In this case, what
kind of tags should I reply? signed-off-by? Reviewed-by? or any other
tags?

Regards,
Yanfei

> Regards,
> Yanfei
>> --
>> RMK's Patch system:
>> https://urldefense.com/v3/__https://www.armlinux.org.uk/developer/patches/__;!!AjveYdw8EvQ!Mz6E44Glfsb6CPijsZ50Ofca9g8UAgL2MRQpaxa18ZTAIrVQpH0IcyqGDwHSWphEoo8$
>> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
>>

2021-05-19 21:08:26

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Tue, 18 May 2021, Nicolas Pitre wrote:

> On Wed, 19 May 2021, Linus Walleij wrote:
>
> > On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
> > <[email protected]> wrote:
> >
> > > Change the current vmalloc_min, which is supposed to be the lowest
> > > address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> > > which does not include VMALLOC_OFFSET.
> > >
> > > Signed-off-by: Russell King (Oracle) <[email protected]>
> >
> > Reviewed-by: Linus Walleij <[email protected]>
> >
> > > +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
> >
> > When I first read this it took me some time to figure out what was
> > going on here, so if you have time, please fold in a comment
> > with some explanation of that (240 << 20) thing, in some blog
> > post I described it as "an interesting way to write 0x0f000000"
> > but I suppose commit 0536bdf33faf chose this way for a
> > specific reason? (Paging Nico if he can explain it.)
>
> That's an alternative (and deprecated) way to write MB(240).

And it seems that MB() isn't globally defined either. Oh well.


Nicolas

2021-05-20 04:56:25

by Alexander Dahl

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform

Hello Yanfei,

Am Wed, May 19, 2021 at 01:32:23PM +0800 schrieb Xu, Yanfei:
> I am not much familar with community contribution. In this case, what
> kind of tags should I reply? signed-off-by? Reviewed-by? or any other
> tags?

This is explained in kernel documentation, starts here with Acked-by:, Cc:,
and Co-developed-by:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

And is followed by Reported-by:, Tested-by:, Reviewed-by:, Suggested-by:,
and Fixes:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Hope that helps.
Alex

2021-05-20 07:08:35

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH] arm: make the size of vmalloc in cmdline and meminfo uniform



On 5/20/21 12:52 PM, Alexander Dahl wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Hello Yanfei,
>
> Am Wed, May 19, 2021 at 01:32:23PM +0800 schrieb Xu, Yanfei:
>> I am not much familar with community contribution. In this case, what
>> kind of tags should I reply? signed-off-by? Reviewed-by? or any other
>> tags?
>
> This is explained in kernel documentation, starts here with Acked-by:, Cc:,
> and Co-developed-by:
>
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/process/submitting-patches.html*when-to-use-acked-by-cc-and-co-developed-by__;Iw!!AjveYdw8EvQ!OL3Mreb08WjGwy5YGRsfsemJBhWkEEIzNVgIGQsyzKYoiF_U2c8ajmauulVY8-lF9N4$
>
> And is followed by Reported-by:, Tested-by:, Reviewed-by:, Suggested-by:,
> and Fixes:
>
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/process/submitting-patches.html*using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes__;Iw!!AjveYdw8EvQ!OL3Mreb08WjGwy5YGRsfsemJBhWkEEIzNVgIGQsyzKYoiF_U2c8ajmauulVYmm6AM_E$
>

These links really help! Thanks for your kind explain. :)

Regards,
Yanfei


> Hope that helps.
> Alex
>

2021-05-20 14:53:53

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Wed, May 19, 2021 at 12:41:10PM +0800, Xu, Yanfei wrote:
> On 5/18/21 8:15 PM, Russell King (Oracle) wrote:
> > -static unsigned long __initdata vmalloc_min =
> > - VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
> > +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
> >
> > /*
> > * vmalloc=size forces the vmalloc area to be exactly 'size'
> > @@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
> > * and may itself be outside the valid range for which phys_addr_t
> > * and therefore __pa() is defined.
> > */
> > - vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
> > + vmalloc_limit = (u64)vmalloc_start -
> > + (PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);
> >
> Here is bug, it should be
>
> vmalloc_limit = (u64)vmalloc_start -
> (PAGE_OFFSET + VMALLOC_OFFSET) + PHYS_OFFSET;

Yes, you're absolutely right, thanks for catching that!

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-05-28 06:03:49

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start



On 5/20/21 5:00 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Wed, May 19, 2021 at 12:41:10PM +0800, Xu, Yanfei wrote:
>> On 5/18/21 8:15 PM, Russell King (Oracle) wrote:
>>> -static unsigned long __initdata vmalloc_min =
>>> - VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
>>> +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
>>>
>>> /*
>>> * vmalloc=size forces the vmalloc area to be exactly 'size'
>>> @@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
>>> * and may itself be outside the valid range for which phys_addr_t
>>> * and therefore __pa() is defined.
>>> */
>>> - vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
>>> + vmalloc_limit = (u64)vmalloc_start -
>>> + (PAGE_OFFSET + PHYS_OFFSET + VMALLOC_OFFSET);
>>>
>> Here is bug, it should be
>>
>> vmalloc_limit = (u64)vmalloc_start -
>> (PAGE_OFFSET + VMALLOC_OFFSET) + PHYS_OFFSET;
>
> Yes, you're absolutely right, thanks for catching that!
>

Hi Russell,

Will you send v2? (or I missed something.)

Thanks,
Yanfei

> --
> RMK's Patch system: https://urldefense.com/v3/__https://www.armlinux.org.uk/developer/patches/__;!!AjveYdw8EvQ!M3h7vuBembpLOcv9xTj5vzxlU0dxXMOa1lDX4yN6u4l-TwnuZ3-ldjYom-7ZQYpsVYc$
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
>

2021-05-28 09:57:27

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Tue, May 18, 2021 at 06:32:43PM -0400, Nicolas Pitre wrote:
> On Tue, 18 May 2021, Nicolas Pitre wrote:
>
> > On Wed, 19 May 2021, Linus Walleij wrote:
> >
> > > On Tue, May 18, 2021 at 2:15 PM Russell King (Oracle)
> > > <[email protected]> wrote:
> > >
> > > > Change the current vmalloc_min, which is supposed to be the lowest
> > > > address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> > > > which does not include VMALLOC_OFFSET.
> > > >
> > > > Signed-off-by: Russell King (Oracle) <[email protected]>
> > >
> > > Reviewed-by: Linus Walleij <[email protected]>
> > >
> > > > +static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
> > >
> > > When I first read this it took me some time to figure out what was
> > > going on here, so if you have time, please fold in a comment
> > > with some explanation of that (240 << 20) thing, in some blog
> > > post I described it as "an interesting way to write 0x0f000000"
> > > but I suppose commit 0536bdf33faf chose this way for a
> > > specific reason? (Paging Nico if he can explain it.)
> >
> > That's an alternative (and deprecated) way to write MB(240).
>
> And it seems that MB() isn't globally defined either. Oh well.

I could add a patch on the end of the series changing it to 240 * SZ_1M
which will likely be clearer.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-05-28 10:02:27

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: change vmalloc_min to vmalloc_start

On Fri, May 28, 2021 at 10:52:28AM +0800, Xu, Yanfei wrote:
> Hi Russell,
>
> Will you send v2? (or I missed something.)

Yes - I've been a little distracted during the last week or so. New
version is in progress.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-05-28 10:14:24

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 3/6] ARM: change vmalloc_min to vmalloc_start

Change the current vmalloc_min, which is supposed to be the lowest
address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
which does not include VMALLOC_OFFSET.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index d932c46a02e0..ed2846bdb1f4 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,8 +1123,7 @@ void __init debug_ll_io_init(void)
}
#endif

-static unsigned long __initdata vmalloc_min =
- VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;
+static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1142,14 +1141,14 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
if (vmalloc_reserve > vmalloc_max) {
vmalloc_reserve = vmalloc_max;
pr_warn("vmalloc area is too big, limiting to %luMB\n",
vmalloc_reserve >> 20);
}

- vmalloc_min = VMALLOC_END - vmalloc_reserve;
+ vmalloc_start = VMALLOC_END - vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1169,7 +1168,8 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
+ vmalloc_limit = (u64)vmalloc_start - VMALLOC_OFFSET -
+ PAGE_OFFSET + PHYS_OFFSET;

/*
* The first usable region must be PMD aligned. Mark its start
--
2.20.1

2021-05-28 10:15:15

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 6/6] ARM: use MiB for vmalloc sizes

Rather than using "m" (which is the unit of metres, or milli), and
"MB" in the printk statements, use MiB to make it clear that we are
talking about the power-of-2 megabytes, aka mebibytes.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index d52647b6261c..a96e9420ec2a 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1128,7 +1128,7 @@ static unsigned long __initdata vmalloc_size = 240 * SZ_1M;
/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
* bytes. This can be used to increase (or decrease) the vmalloc
- * area - the default is 240m.
+ * area - the default is 240MiB.
*/
static int __init early_vmalloc(char *arg)
{
@@ -1137,14 +1137,14 @@ static int __init early_vmalloc(char *arg)

if (vmalloc_reserve < SZ_16M) {
vmalloc_reserve = SZ_16M;
- pr_warn("vmalloc area too small, limiting to %luMB\n",
+ pr_warn("vmalloc area is too small, limiting to %luMiB\n",
vmalloc_reserve >> 20);
}

vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
if (vmalloc_reserve > vmalloc_max) {
vmalloc_reserve = vmalloc_max;
- pr_warn("vmalloc area is too big, limiting to %luMB\n",
+ pr_warn("vmalloc area is too big, limiting to %luMiB\n",
vmalloc_reserve >> 20);
}

--
2.20.1

2021-05-28 10:17:04

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 4/6] ARM: change vmalloc_start to vmalloc_size

Rather than storing the start of vmalloc space, store the size, and
move the calculation into adjust_lowmem_limit(). We now have one single
place where this calculation takes place.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index ed2846bdb1f4..5ae11e6f2a58 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,7 +1123,7 @@ void __init debug_ll_io_init(void)
}
#endif

-static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
+static unsigned long __initdata vmalloc_size = 240 << 20;

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1148,7 +1148,7 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_start = VMALLOC_END - vmalloc_reserve;
+ vmalloc_size = vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1168,7 +1168,7 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)vmalloc_start - VMALLOC_OFFSET -
+ vmalloc_limit = (u64)VMALLOC_END - vmalloc_size - VMALLOC_OFFSET -
PAGE_OFFSET + PHYS_OFFSET;

/*
--
2.20.1

2021-05-28 10:17:06

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 5/6] ARM: use "* SZ_1M" rather than "<< 20"

Make the default vmalloc size clearer by using a more natural
multiplication by SZ_1M rather than a shift left by 20 bits.

Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 5ae11e6f2a58..d52647b6261c 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,7 +1123,7 @@ void __init debug_ll_io_init(void)
}
#endif

-static unsigned long __initdata vmalloc_size = 240 << 20;
+static unsigned long __initdata vmalloc_size = 240 * SZ_1M;

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
--
2.20.1

2021-05-28 12:13:09

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 2/6] ARM: use a temporary variable to hold maximum vmalloc size

We calculate the maximum size of the vmalloc space twice in
early_vmalloc(). Use a temporary variable to hold this value.

Reviewed-by: Linus Walleij <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 206c345f063e..d932c46a02e0 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1134,6 +1134,7 @@ static unsigned long __initdata vmalloc_min =
static int __init early_vmalloc(char *arg)
{
unsigned long vmalloc_reserve = memparse(arg, NULL);
+ unsigned long vmalloc_max;

if (vmalloc_reserve < SZ_16M) {
vmalloc_reserve = SZ_16M;
@@ -1141,8 +1142,9 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- if (vmalloc_reserve > VMALLOC_END - (PAGE_OFFSET + SZ_32M)) {
- vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
+ if (vmalloc_reserve > vmalloc_max) {
+ vmalloc_reserve = vmalloc_max;
pr_warn("vmalloc area is too big, limiting to %luMB\n",
vmalloc_reserve >> 20);
}
--
2.20.1

2021-05-28 12:13:24

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH v2 1/6] ARM: change vmalloc_min to be unsigned long

vmalloc_min is currently a void pointer, but everywhere its used
contains a cast - either to a void pointer when setting or back to
an integer type when being used. Eliminate these casts by changing
its type to unsigned long.

Reviewed-by: Linus Walleij <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm/mm/mmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index c06ebfbc48c4..206c345f063e 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1123,8 +1123,8 @@ void __init debug_ll_io_init(void)
}
#endif

-static void * __initdata vmalloc_min =
- (void *)(VMALLOC_END - (240 << 20) - VMALLOC_OFFSET);
+static unsigned long __initdata vmalloc_min =
+ VMALLOC_END - (240 << 20) - VMALLOC_OFFSET;

/*
* vmalloc=size forces the vmalloc area to be exactly 'size'
@@ -1147,7 +1147,7 @@ static int __init early_vmalloc(char *arg)
vmalloc_reserve >> 20);
}

- vmalloc_min = (void *)(VMALLOC_END - vmalloc_reserve);
+ vmalloc_min = VMALLOC_END - vmalloc_reserve;
return 0;
}
early_param("vmalloc", early_vmalloc);
@@ -1167,7 +1167,7 @@ void __init adjust_lowmem_bounds(void)
* and may itself be outside the valid range for which phys_addr_t
* and therefore __pa() is defined.
*/
- vmalloc_limit = (u64)(uintptr_t)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
+ vmalloc_limit = (u64)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;

/*
* The first usable region must be PMD aligned. Mark its start
--
2.20.1

2021-05-28 12:43:09

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] ARM: use MiB for vmalloc sizes

On Fri, May 28, 2021 at 12:11 PM Russell King (Oracle)
<[email protected]> wrote:
>
> Rather than using "m" (which is the unit of metres, or milli), and
> "MB" in the printk statements, use MiB to make it clear that we are
> talking about the power-of-2 megabytes, aka mebibytes.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-28 12:45:01

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] ARM: change vmalloc_min to vmalloc_start

On Fri, May 28, 2021 at 12:11 PM Russell King (Oracle)
<[email protected]> wrote:

> Change the current vmalloc_min, which is supposed to be the lowest
> address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> which does not include VMALLOC_OFFSET.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-28 12:45:35

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] ARM: use "* SZ_1M" rather than "<< 20"

On Fri, May 28, 2021 at 12:11 PM Russell King (Oracle)
<[email protected]> wrote:

> Make the default vmalloc size clearer by using a more natural
> multiplication by SZ_1M rather than a shift left by 20 bits.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Excellent, thanks!
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-28 13:14:02

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] ARM: use a temporary variable to hold maximum vmalloc size



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> We calculate the maximum size of the vmalloc space twice in
> early_vmalloc(). Use a temporary variable to hold this value.
>
> Reviewed-by: Linus Walleij <[email protected]>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Regards,
Yanfei
> ---
> arch/arm/mm/mmu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 206c345f063e..d932c46a02e0 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1134,6 +1134,7 @@ static unsigned long __initdata vmalloc_min =
> static int __init early_vmalloc(char *arg)
> {
> unsigned long vmalloc_reserve = memparse(arg, NULL);
> + unsigned long vmalloc_max;
>
> if (vmalloc_reserve < SZ_16M) {
> vmalloc_reserve = SZ_16M;
> @@ -1141,8 +1142,9 @@ static int __init early_vmalloc(char *arg)
> vmalloc_reserve >> 20);
> }
>
> - if (vmalloc_reserve > VMALLOC_END - (PAGE_OFFSET + SZ_32M)) {
> - vmalloc_reserve = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
> + vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M);
> + if (vmalloc_reserve > vmalloc_max) {
> + vmalloc_reserve = vmalloc_max;
> pr_warn("vmalloc area is too big, limiting to %luMB\n",
> vmalloc_reserve >> 20);
> }
> --
> 2.20.1
>

2021-05-28 14:10:27

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] ARM: change vmalloc_min to vmalloc_start



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> Change the current vmalloc_min, which is supposed to be the lowest
> address of vmalloc space including the VMALLOC_OFFSET, to vmalloc_start
> which does not include VMALLOC_OFFSET.
>
> Signed-off-by: Russell King (Oracle)<[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Thanks!


Regards,
Yanfei

2021-05-28 14:17:58

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] ARM: change vmalloc_start to vmalloc_size



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Rather than storing the start of vmalloc space, store the size, and
> move the calculation into adjust_lowmem_limit(). We now have one single
> place where this calculation takes place.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Regards,
Yanfei
> ---
> arch/arm/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index ed2846bdb1f4..5ae11e6f2a58 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1123,7 +1123,7 @@ void __init debug_ll_io_init(void)
> }
> #endif
>
> -static unsigned long __initdata vmalloc_start = VMALLOC_END - (240 << 20);
> +static unsigned long __initdata vmalloc_size = 240 << 20;
>
> /*
> * vmalloc=size forces the vmalloc area to be exactly 'size'
> @@ -1148,7 +1148,7 @@ static int __init early_vmalloc(char *arg)
> vmalloc_reserve >> 20);
> }
>
> - vmalloc_start = VMALLOC_END - vmalloc_reserve;
> + vmalloc_size = vmalloc_reserve;
> return 0;
> }
> early_param("vmalloc", early_vmalloc);
> @@ -1168,7 +1168,7 @@ void __init adjust_lowmem_bounds(void)
> * and may itself be outside the valid range for which phys_addr_t
> * and therefore __pa() is defined.
> */
> - vmalloc_limit = (u64)vmalloc_start - VMALLOC_OFFSET -
> + vmalloc_limit = (u64)VMALLOC_END - vmalloc_size - VMALLOC_OFFSET -
> PAGE_OFFSET + PHYS_OFFSET;
>
> /*
> --
> 2.20.1
>

2021-05-28 15:09:48

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] ARM: change vmalloc_start to vmalloc_size

On Fri, May 28, 2021 at 12:11 PM Russell King (Oracle)
<[email protected]> wrote:

> Rather than storing the start of vmalloc space, store the size, and
> move the calculation into adjust_lowmem_limit(). We now have one single
> place where this calculation takes place.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-05-28 15:12:22

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] ARM: change vmalloc_min to be unsigned long



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> vmalloc_min is currently a void pointer, but everywhere its used
> contains a cast - either to a void pointer when setting or back to
> an integer type when being used. Eliminate these casts by changing
> its type to unsigned long.
>
> Reviewed-by: Linus Walleij<[email protected]>
> Signed-off-by: Russell King (Oracle)<[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Regards,
Yanfei

2021-05-28 15:27:33

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] ARM: use MiB for vmalloc sizes



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Rather than using "m" (which is the unit of metres, or milli), and
> "MB" in the printk statements, use MiB to make it clear that we are
> talking about the power-of-2 megabytes, aka mebibytes.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Regards,
Yanfei

> ---
> arch/arm/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index d52647b6261c..a96e9420ec2a 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1128,7 +1128,7 @@ static unsigned long __initdata vmalloc_size = 240 * SZ_1M;
> /*
> * vmalloc=size forces the vmalloc area to be exactly 'size'
> * bytes. This can be used to increase (or decrease) the vmalloc
> - * area - the default is 240m.
> + * area - the default is 240MiB.
> */
> static int __init early_vmalloc(char *arg)
> {
> @@ -1137,14 +1137,14 @@ static int __init early_vmalloc(char *arg)
>
> if (vmalloc_reserve < SZ_16M) {
> vmalloc_reserve = SZ_16M;
> - pr_warn("vmalloc area too small, limiting to %luMB\n",
> + pr_warn("vmalloc area is too small, limiting to %luMiB\n",
> vmalloc_reserve >> 20);
> }
>
> vmalloc_max = VMALLOC_END - (PAGE_OFFSET + SZ_32M + VMALLOC_OFFSET);
> if (vmalloc_reserve > vmalloc_max) {
> vmalloc_reserve = vmalloc_max;
> - pr_warn("vmalloc area is too big, limiting to %luMB\n",
> + pr_warn("vmalloc area is too big, limiting to %luMiB\n",
> vmalloc_reserve >> 20);
> }
>
> --
> 2.20.1
>

2021-05-28 15:27:33

by Xu, Yanfei

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] ARM: use "* SZ_1M" rather than "<< 20"



On 5/28/21 6:11 PM, Russell King (Oracle) wrote:
> Make the default vmalloc size clearer by using a more natural
> multiplication by SZ_1M rather than a shift left by 20 bits.
>
> Signed-off-by: Russell King (Oracle)<[email protected]>

Reviewed-by: Yanfei Xu <[email protected]>

Regards,
Yanfei

> ---