| Message ID | 163166721835.510331.4931010992364519157.stgit@devnote2 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bootconfig: Fixes to bootconfig memory management | expand |
On Tue, Sep 14, 2021 at 5:53 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Since commit 77e02cf57b6c ("memblock: introduce saner > 'memblock_free_ptr()' interface") introduced memblock_free_ptr() > to lib/bootconfig.c, bootconfig tool also has to define > memblock_free_ptr() wrapper, and remove unused __pa() and > memblock_free(). Christ. I grepped for this, and couldn't find any use of that memblock_free function in the tools directory, so I ignored it. It seems like the code in lib/bootconfig.c is compiled both into the kernel and into that tool. This is a nightmare. We've explicitly tried to avoid this for the tooling headers exactly because of issues like this. Linus
On Tue, 14 Sep 2021 18:21:09 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Sep 14, 2021 at 5:53 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > Since commit 77e02cf57b6c ("memblock: introduce saner > > 'memblock_free_ptr()' interface") introduced memblock_free_ptr() > > to lib/bootconfig.c, bootconfig tool also has to define > > memblock_free_ptr() wrapper, and remove unused __pa() and > > memblock_free(). > > Christ. > > I grepped for this, and couldn't find any use of that memblock_free > function in the tools directory, so I ignored it. > > It seems like the code in lib/bootconfig.c is compiled both into the > kernel and into that tool. This is a nightmare. We've explicitly tried > to avoid this for the tooling headers exactly because of issues like > this. Hmm, OK. Let me copy lib/bootconfig.c itself into tools/bootconfig as a user-space code. Thank you, > > Linus
On Tue, Sep 14, 2021 at 6:47 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hmm, OK. Let me copy lib/bootconfig.c itself into tools/bootconfig > as a user-space code. Well, or we need to have some really good way to mark these shared files. Normally I don't think we share any *.c files with tooling, and tooling copies over the *.h files it needs. Is this the only one? So yes, copying the *.c file in this case would match what we do for the header files, but particularly if there are others, maybe we could have something like the "uapi" directory that allows people to explicialy share files with the tools. But it would need to be very explicit in the pathname, so that people would have that big warning sign of "hey, now you're editing a file that is shared with tooling". That has worked at least _somewhat_ with include/uapi/ and arch/*/include/uapi/. Linus
On Tue, 14 Sep 2021 18:57:34 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Sep 14, 2021 at 6:47 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > Hmm, OK. Let me copy lib/bootconfig.c itself into tools/bootconfig > > as a user-space code. > > Well, or we need to have some really good way to mark these shared files. > > Normally I don't think we share any *.c files with tooling, and > tooling copies over the *.h files it needs. Is this the only one? What I need to share is lib/bootconfig.c and include/linux/bootconfig.h. Those provides bootconfig APIs and parser. But since bootconfig.c uses some kernel APIs, I made wrapper header files. If I can add #ifdefs to split the parser and APIs from those part (as you can see 90% of code doesn't need kernel APIs), I think I don't need any wrappers to include the file (instead of copying bootconfig.c). > So yes, copying the *.c file in this case would match what we do for > the header files, but particularly if there are others, maybe we could > have something like the "uapi" directory that allows people to > explicialy share files with the tools. OK. > But it would need to be very explicit in the pathname, so that people > would have that big warning sign of "hey, now you're editing a file > that is shared with tooling". > > That has worked at least _somewhat_ with include/uapi/ and arch/*/include/uapi/. Hmm, what about lib/uapi/bootconfig.c ? Thank you, > > Linus
diff --git a/tools/bootconfig/include/linux/memblock.h b/tools/bootconfig/include/linux/memblock.h index 7862f217d85d..f2e506f7d57f 100644 --- a/tools/bootconfig/include/linux/memblock.h +++ b/tools/bootconfig/include/linux/memblock.h @@ -4,9 +4,8 @@ #include <stdlib.h> -#define __pa(addr) (addr) #define SMP_CACHE_BYTES 0 #define memblock_alloc(size, align) malloc(size) -#define memblock_free(paddr, size) free(paddr) +#define memblock_free_ptr(paddr, size) free(paddr) #endif
Since commit 77e02cf57b6c ("memblock: introduce saner 'memblock_free_ptr()' interface") introduced memblock_free_ptr() to lib/bootconfig.c, bootconfig tool also has to define memblock_free_ptr() wrapper, and remove unused __pa() and memblock_free(). Fixes: 77e02cf57b6c ("memblock: introduce saner 'memblock_free_ptr()' interface") Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/bootconfig/include/linux/memblock.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)