Skip to contentSkip to conversation

arch/um: fix forward declaration for vmalloc

PATCH In review Suren Baghdasaryan 4 participants 8 messages Mar 26, 2024 – Apr 22, 2024

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arch/um: fix forward declaration for vmalloc

Patch [1] replaced vmalloc() function with a new definition but it did not adjust the forward declaration used in UML architecture. Change it to act as before.
Note that this prevents the vmalloc() allocations in __wrap_malloc() from being accounted. If accounting here is critical, we will have to remove this forward declaration and include vmalloc.h, however that would pull in more dependencies and would require introducing more architecture-specific headers, like asm/bug.h, asm/rwonce.h, etc. This is likely the reason why this forward declaration was introduced in the first place.

[1] https://lore.kernel.org/all/[email protected]/

Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")
---
 arch/um/include/shared/um_malloc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/um/include/shared/um_malloc.h +2 -1
index 13da93284c2c..bf503658f08e 100644
@@ -11,7 +11,8 @@
 extern void *uml_kmalloc(int size, int flags);
 extern void kfree(const void *ptr);
 
-extern void *vmalloc(unsigned long size);
+extern void *vmalloc_noprof(unsigned long size);
+#define vmalloc(...)		vmalloc_noprof(__VA_ARGS__)
 extern void vfree(void *ptr);
 
 #endif /* __UM_MALLOC_H__ */
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
Tested-by on v1 1/1
#1 lore raw
Quoted 11 lines · On Tue, 26 Mar 2024 00:37:50 -0700 Suren Baghdasaryan <[email protected]> wrote:

Patch [1] replaced vmalloc() function with a new definition but it did not adjust the forward declaration used in UML architecture. Change it to act as before.
Note that this prevents the vmalloc() allocations in __wrap_malloc() from being accounted. If accounting here is critical, we will have to remove this forward declaration and include vmalloc.h, however that would pull in more dependencies and would require introducing more architecture-specific headers, like asm/bug.h, asm/rwonce.h, etc. This is likely the reason why this forward declaration was introduced in the first place.

Quoted 5 lines

[1] https://lore.kernel.org/all/[email protected]/

Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")

Thank you for this fix, Suren. I confirmed that this patch fixes the issue I reported.

Thanks,
SJ

Quoted 20 lines
---
 arch/um/include/shared/um_malloc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/um/include/shared/um_malloc.h +2 -1
index 13da93284c2c..bf503658f08e 100644
@@ -11,7 +11,8 @@
 extern void *uml_kmalloc(int size, int flags);
 extern void kfree(const void *ptr);
 
-extern void *vmalloc(unsigned long size);
+extern void *vmalloc_noprof(unsigned long size);
+#define vmalloc(...)		vmalloc_noprof(__VA_ARGS__)
 extern void vfree(void *ptr);
 
 #endif /* __UM_MALLOC_H__ */
Signature · 1 line
2.44.0.396.g6e790dbe36-goog
maintainer Reviewed-by on v1 1/1
#2 lore raw
In reply to Suren Baghdasaryan

arch/um/include/shared/um_malloc.h line 15

Quoted 5 lines · Suren Baghdasaryan
arch/um/include/shared/um_malloc.h +2 -1
-extern void *vmalloc(unsigned long size);
+extern void *vmalloc_noprof(unsigned long size);
+#define vmalloc(...)		vmalloc_noprof(__VA_ARGS__)

I was confused a bit by the define at first, but that's because this is a user-side header file.

johannes

#3 lore raw
In reply to Suren Baghdasaryan

----- Ursprüngliche Mail -----

Quoted 14 lines

Von: "Suren Baghdasaryan" <[email protected]>
Betreff: [PATCH 1/1] arch/um: fix forward declaration for vmalloc

Patch [1] replaced vmalloc() function with a new definition but it did not adjust the forward declaration used in UML architecture. Change it to act as before.
Note that this prevents the vmalloc() allocations in __wrap_malloc() from being accounted. If accounting here is critical, we will have to remove this forward declaration and include vmalloc.h, however that would pull in more dependencies and would require introducing more architecture-specific headers, like asm/bug.h, asm/rwonce.h, etc. This is likely the reason why this forward declaration was introduced in the first place.

Quoted 3 lines

[1] https://lore.kernel.org/all/[email protected]/

Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")

This commit id is not in Linus tree.
Do I miss something?

Thanks,
//richard

#4 lore raw
Quoted 16 lines · Richard Weinberger

----- Ursprüngliche Mail -----

Quoted 14 lines

Von: "Suren Baghdasaryan" <[email protected]>
Betreff: [PATCH 1/1] arch/um: fix forward declaration for vmalloc

Patch [1] replaced vmalloc() function with a new definition but it did not adjust the forward declaration used in UML architecture. Change it to act as before.
Note that this prevents the vmalloc() allocations in __wrap_malloc() from being accounted. If accounting here is critical, we will have to remove this forward declaration and include vmalloc.h, however that would pull in more dependencies and would require introducing more architecture-specific headers, like asm/bug.h, asm/rwonce.h, etc. This is likely the reason why this forward declaration was introduced in the first place.

Quoted 6 lines
Quoted 3 lines

[1] https://lore.kernel.org/all/[email protected]/

Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")

This commit id is not in Linus tree.
Do I miss something?

It's in mm-unstable under dc26c7e79daf2fc11169b23c150862f0e878ee5a. I think it just didn't reach Linus' tree yet.

Quoted 3 lines

Thanks,
//richard

#5 lore raw

----- Ursprüngliche Mail -----

Quoted 8 lines

Von: "Suren Baghdasaryan" <[email protected]>

Quoted 4 lines
Quoted 1 line
Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")

This commit id is not in Linus tree.
Do I miss something?

It's in mm-unstable under dc26c7e79daf2fc11169b23c150862f0e878ee5a. I think it just didn't reach Linus' tree yet.

Hmm, so we better postpone this path until said commit hits Linus tree, or you carry it together with the commit in mm-unstable.

Thanks,
//richard

#6 lore raw
Quoted 5 lines · Richard Weinberger

----- Ursprüngliche Mail -----

Quoted 3 lines

Von: "Suren Baghdasaryan" <[email protected]>

Quoted 2 lines
Quoted 1 line
Fixes 576477564ede ("mm: vmalloc: enable memory allocation profiling")
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

This commit id is not in Linus tree.
Do I miss something?

It's in mm-unstable under dc26c7e79daf2fc11169b23c150862f0e878ee5a. I think it just didn't reach Linus' tree yet.

Hmm, so we better postpone this path until said commit hits Linus tree, or you carry it together with the commit in mm-unstable.

Oh, sorry, I didn't realize you were talking about the `Fixes:
576477564ede` part... Yeah, unfortunately SHAs in mm-unstable are unstable, so the change being fixed is under edf0a25633bda1e5b7844478dd13b4326a3d5d09 now. I think Andrew placed this fix right after the change it fixes with intention to merge them together later on.

Quoted 3 lines

Thanks,
//richard

maintainer Acked-by
#7 lore raw

----- Ursprüngliche Mail -----

Quoted 7 lines

Von: "Suren Baghdasaryan" <[email protected]>

Quoted 5 lines
Quoted 2 lines

It's in mm-unstable under dc26c7e79daf2fc11169b23c150862f0e878ee5a. I think it just didn't reach Linus' tree yet.

Hmm, so we better postpone this path until said commit hits Linus tree, or you carry it together with the commit in mm-unstable.

Quoted 6 lines

Oh, sorry, I didn't realize you were talking about the `Fixes:
576477564ede` part... Yeah, unfortunately SHAs in mm-unstable are unstable, so the change being fixed is under edf0a25633bda1e5b7844478dd13b4326a3d5d09 now. I think Andrew placed this fix right after the change it fixes with intention to merge them together later on.

Ah, the patch itself goes via Andrew's tree, works for me! Let me note this in the uml patchwork system.

Thanks,
//richard