find_vmap_lowest_match() is now able to handle different roots. Make
similar changes to find_vmap_lowest_match_check() and
find_vmap_lowest_linear_match() to handle different trees.
Fixes: f9863be49312 ("mm/vmalloc: extend __alloc_vmap_area() with extra arguments")
Cc: Uladzislau Rezki (Sony) <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Song Liu <[email protected]>
---
mm/vmalloc.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index dd6cdb201195..088b421601c4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1300,12 +1300,12 @@ find_vmap_lowest_match(struct rb_root *root, unsigned long size,
#include <linux/random.h>
static struct vmap_area *
-find_vmap_lowest_linear_match(unsigned long size,
+find_vmap_lowest_linear_match(struct list_head *head, unsigned long size,
unsigned long align, unsigned long vstart)
{
struct vmap_area *va;
- list_for_each_entry(va, &free_vmap_area_list, list) {
+ list_for_each_entry(va, head, list) {
if (!is_within_this_va(va, size, align, vstart))
continue;
@@ -1316,7 +1316,8 @@ find_vmap_lowest_linear_match(unsigned long size,
}
static void
-find_vmap_lowest_match_check(unsigned long size, unsigned long align)
+find_vmap_lowest_match_check(struct rb_root *root, struct list_head *head,
+ unsigned long size, unsigned long align)
{
struct vmap_area *va_1, *va_2;
unsigned long vstart;
@@ -1325,8 +1326,8 @@ find_vmap_lowest_match_check(unsigned long size, unsigned long align)
get_random_bytes(&rnd, sizeof(rnd));
vstart = VMALLOC_START + rnd;
- va_1 = find_vmap_lowest_match(size, align, vstart, false);
- va_2 = find_vmap_lowest_linear_match(size, align, vstart);
+ va_1 = find_vmap_lowest_match(root, size, align, vstart, false);
+ va_2 = find_vmap_lowest_linear_match(head, size, align, vstart);
if (va_1 != va_2)
pr_emerg("not lowest: t: 0x%p, l: 0x%p, v: 0x%lx\n",
@@ -1513,7 +1514,7 @@ __alloc_vmap_area(struct rb_root *root, struct list_head *head,
return vend;
#if DEBUG_AUGMENT_LOWEST_MATCH_CHECK
- find_vmap_lowest_match_check(size, align);
+ find_vmap_lowest_match_check(root, head, size, align);
#endif
return nva_start_addr;
--
2.30.2
On 08/30/22 at 10:27pm, Song Liu wrote:
> find_vmap_lowest_match() is now able to handle different roots. Make
> similar changes to find_vmap_lowest_match_check() and
> find_vmap_lowest_linear_match() to handle different trees.
Good catch. Guess usually nobody eables DEBUG_AUGMENT_LOWEST_MATCH_CHECK
to trigger compilation of the code.
Reviewed-by: Baoquan He <[email protected]>
>
> Fixes: f9863be49312 ("mm/vmalloc: extend __alloc_vmap_area() with extra arguments")
> Cc: Uladzislau Rezki (Sony) <[email protected]>
> Cc: Baoquan He <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Song Liu <[email protected]>
> ---
> mm/vmalloc.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index dd6cdb201195..088b421601c4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1300,12 +1300,12 @@ find_vmap_lowest_match(struct rb_root *root, unsigned long size,
> #include <linux/random.h>
>
> static struct vmap_area *
> -find_vmap_lowest_linear_match(unsigned long size,
> +find_vmap_lowest_linear_match(struct list_head *head, unsigned long size,
> unsigned long align, unsigned long vstart)
> {
> struct vmap_area *va;
>
> - list_for_each_entry(va, &free_vmap_area_list, list) {
> + list_for_each_entry(va, head, list) {
> if (!is_within_this_va(va, size, align, vstart))
> continue;
>
> @@ -1316,7 +1316,8 @@ find_vmap_lowest_linear_match(unsigned long size,
> }
>
> static void
> -find_vmap_lowest_match_check(unsigned long size, unsigned long align)
> +find_vmap_lowest_match_check(struct rb_root *root, struct list_head *head,
> + unsigned long size, unsigned long align)
> {
> struct vmap_area *va_1, *va_2;
> unsigned long vstart;
> @@ -1325,8 +1326,8 @@ find_vmap_lowest_match_check(unsigned long size, unsigned long align)
> get_random_bytes(&rnd, sizeof(rnd));
> vstart = VMALLOC_START + rnd;
>
> - va_1 = find_vmap_lowest_match(size, align, vstart, false);
> - va_2 = find_vmap_lowest_linear_match(size, align, vstart);
> + va_1 = find_vmap_lowest_match(root, size, align, vstart, false);
> + va_2 = find_vmap_lowest_linear_match(head, size, align, vstart);
>
> if (va_1 != va_2)
> pr_emerg("not lowest: t: 0x%p, l: 0x%p, v: 0x%lx\n",
> @@ -1513,7 +1514,7 @@ __alloc_vmap_area(struct rb_root *root, struct list_head *head,
> return vend;
>
> #if DEBUG_AUGMENT_LOWEST_MATCH_CHECK
> - find_vmap_lowest_match_check(size, align);
> + find_vmap_lowest_match_check(root, head, size, align);
> #endif
>
> return nva_start_addr;
> --
> 2.30.2
>
> On 08/30/22 at 10:27pm, Song Liu wrote:
> > find_vmap_lowest_match() is now able to handle different roots. Make
> > similar changes to find_vmap_lowest_match_check() and
> > find_vmap_lowest_linear_match() to handle different trees.
>
> Good catch. Guess usually nobody eables DEBUG_AUGMENT_LOWEST_MATCH_CHECK
> to trigger compilation of the code.
>
> Reviewed-by: Baoquan He <[email protected]>
>
I do it.
Reviewed-by: Uladzislau Rezki (Sony) <[email protected]>
--
Uladzislau Rezki
On Tue, 30 Aug 2022 22:27:34 -0700 Song Liu <[email protected]> wrote:
> find_vmap_lowest_match() is now able to handle different roots. Make
> similar changes to find_vmap_lowest_match_check() and
> find_vmap_lowest_linear_match() to handle different trees.
What are the runtime effects of this change?
On Wed, Aug 31, 2022 at 4:01 PM Andrew Morton <[email protected]> wrote:
>
> On Tue, 30 Aug 2022 22:27:34 -0700 Song Liu <[email protected]> wrote:
>
> > find_vmap_lowest_match() is now able to handle different roots. Make
> > similar changes to find_vmap_lowest_match_check() and
> > find_vmap_lowest_linear_match() to handle different trees.
>
> What are the runtime effects of this change?
The code is gated by DEBUG_AUGMENT_LOWEST_MATCH_CHECK. It
is only compiled when the developer enables it explicitly. Therefore,
there isn't
any runtime effect.
Thanks,
Song
This is only for debug purpose.
Even without this patch, the debug path would work correctly. The
difference is just only in
whether roots are hardcoded or passed over function paramter.
--
Uladzislau Rezki
On Thu, Sep 1, 2022 at 2:47 AM Song Liu <[email protected]> wrote:
>
> On Wed, Aug 31, 2022 at 4:01 PM Andrew Morton <[email protected]> wrote:
> >
> > On Tue, 30 Aug 2022 22:27:34 -0700 Song Liu <[email protected]> wrote:
> >
> > > find_vmap_lowest_match() is now able to handle different roots. Make
> > > similar changes to find_vmap_lowest_match_check() and
> > > find_vmap_lowest_linear_match() to handle different trees.
> >
> > What are the runtime effects of this change?
>
> The code is gated by DEBUG_AUGMENT_LOWEST_MATCH_CHECK. It
> is only compiled when the developer enables it explicitly. Therefore,
> there isn't
> any runtime effect.
>
> Thanks,
> Song
--
Uladzislau Rezki
On 09/01/22 at 02:16pm, Uladzislau Rezki wrote:
> This is only for debug purpose.
>
> Even without this patch, the debug path would work correctly. The
> difference is just only in
> whether roots are hardcoded or passed over function paramter.
Calling find_vmap_lowest_match() inside find_vmap_lowest_match_check()
will fail compilation because the function interface has been changed.
>
> On Thu, Sep 1, 2022 at 2:47 AM Song Liu <[email protected]> wrote:
> >
> > On Wed, Aug 31, 2022 at 4:01 PM Andrew Morton <[email protected]> wrote:
> > >
> > > On Tue, 30 Aug 2022 22:27:34 -0700 Song Liu <[email protected]> wrote:
> > >
> > > > find_vmap_lowest_match() is now able to handle different roots. Make
> > > > similar changes to find_vmap_lowest_match_check() and
> > > > find_vmap_lowest_linear_match() to handle different trees.
> > >
> > > What are the runtime effects of this change?
> >
> > The code is gated by DEBUG_AUGMENT_LOWEST_MATCH_CHECK. It
> > is only compiled when the developer enables it explicitly. Therefore,
> > there isn't
> > any runtime effect.
> >
> > Thanks,
> > Song
>
>
>
> --
> Uladzislau Rezki
>
> > Even without this patch, the debug path would work correctly. The
> > difference is just only in
> > whether roots are hardcoded or passed over function paramter.
>
> Calling find_vmap_lowest_match() inside find_vmap_lowest_match_check()
> will fail compilation because the function interface has been changed.
>
Ah. That makes sense, though the commit message has to reflect it.
So it is only about compilation error if debug is ON.
--
Uladzislau Rezki
On 09/02/22 at 06:45pm, Uladzislau Rezki wrote:
> > > Even without this patch, the debug path would work correctly. The
> > > difference is just only in
> > > whether roots are hardcoded or passed over function paramter.
> >
> > Calling find_vmap_lowest_match() inside find_vmap_lowest_match_check()
> > will fail compilation because the function interface has been changed.
> >
> Ah. That makes sense, though the commit message has to reflect it.
> So it is only about compilation error if debug is ON.
Indeed, the current patch log sounds like an improvement or normal change.
In fact it's a code fix.
> On 09/02/22 at 06:45pm, Uladzislau Rezki wrote:
> > > > Even without this patch, the debug path would work correctly. The
> > > > difference is just only in
> > > > whether roots are hardcoded or passed over function paramter.
> > >
> > > Calling find_vmap_lowest_match() inside find_vmap_lowest_match_check()
> > > will fail compilation because the function interface has been changed.
> > >
> > Ah. That makes sense, though the commit message has to reflect it.
> > So it is only about compilation error if debug is ON.
>
> Indeed, the current patch log sounds like an improvement or normal change.
> In fact it's a code fix.
>
Then i think it is worth to mention about this in the commit message. At
least i have missed the main point of this change looking at the commit
message.
Song Liu, Could you please upload a v2 of it stating exactly what it fixes?
<snip>
urezki@pc638:~/data/raid0/coding/linux-next.git$ git diff
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e68c0081e861..7552f1f8350e 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -713,7 +713,7 @@ EXPORT_SYMBOL(vmalloc_to_pfn);
/*** Global kva allocator ***/
#define DEBUG_AUGMENT_PROPAGATE_CHECK 0
-#define DEBUG_AUGMENT_LOWEST_MATCH_CHECK 0
+#define DEBUG_AUGMENT_LOWEST_MATCH_CHECK 1
static DEFINE_SPINLOCK(vmap_area_lock);
urezki@pc638:~/data/raid0/coding/linux-next.git$ make -j64 bzImage
DESCEND objtool
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
CC mm/vmalloc.o
mm/vmalloc.c: In function ‘find_vmap_lowest_match_check’:
mm/vmalloc.c:1328:32: warning: passing argument 1 of ‘find_vmap_lowest_match’ makes pointer from integer without a cast [-Wint-conversion]
1328 | va_1 = find_vmap_lowest_match(size, align, vstart, false);
| ^~~~
| |
| long unsigned int
mm/vmalloc.c:1236:40: note: expected ‘struct rb_root *’ but argument is of type ‘long unsigned int’
1236 | find_vmap_lowest_match(struct rb_root *root, unsigned long size,
| ~~~~~~~~~~~~~~~~^~~~
mm/vmalloc.c:1328:9: error: too few arguments to function ‘find_vmap_lowest_match’
1328 | va_1 = find_vmap_lowest_match(size, align, vstart, false);
| ^~~~~~~~~~~~~~~~~~~~~~
mm/vmalloc.c:1236:1: note: declared here
1236 | find_vmap_lowest_match(struct rb_root *root, unsigned long size,
| ^~~~~~~~~~~~~~~~~~~~~~
make[1]: *** [scripts/Makefile.build:250: mm/vmalloc.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:2003: mm] Error 2
make: *** Waiting for unfinished jobs....
urezki@pc638:~/data/raid0/coding/linux-next.git$
<snip>
Thank you in advance!
--
Uladzislau Rezki