| Message ID | ef34ee3cc8cb0e4f8ce7c7c9975a0e8d14473a84.1688828139.git.falcon@tinylab.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: shrink arch support | expand |
On Sat, Jul 08, 2023 at 11:26:42PM +0800, Zhangjin Wu wrote: > Currently, the architecture specific arch.h has two parts, one is the > syscall declarations for sys.h, another is the _start code definition > for startup support. > > The coming crt.h will provide the startup support with a new common > _start_c(), it will replace most of the assembly _start code and shrink > the original _start code to be minimal, as a result, _start_c() and the > left minimal _start code will work together to provide the startup > support, therefore, the left _start code will be only required by crt.h. > > So, the syscall declarations part of arch.h can be split to sys_arch.h > and the _start code part of arch.h can be split to crt_arch.h and then, > they should only be included in sys.h and crt.h respectively. > > At the same time, the architecture specific arch-<ARCH>.h should be > split to <ARCH>/crt.h and <ARCH>/sys.h. > > As a preparation, this creates the architecture specific directory and > moves tools/include/nolibc/arch-<ARCH>.h to > tools/include/nolibc/<ARCH>/arch.h. I'm sorry but I still don't understand what it *provides*. I'm reading it as "we *can* do this so let's do it". But what is the specific purpose of adding this extra directory structure ? It's really unclear to me and worries me that it'll only result in complicating maintenance by adding even more files, thus even more "include" lines and cross dependencies. Zhangjin, very often in your series, the justification for a change is missing, instead it's only explaining what is being changed, and I must confess that it makes it particularly difficult to figure the benefits. I'm only seeing this as an opportunity for a change ("can be split"). I could have missed something of course, but I can't figure what problem it is trying to solve. As a general advice, I tend to remind people that when sending a patch series, they should consider they're trying to sell it, so they must emphasize the benefits of accepting the series for the maintainer(s). You very likely have a good reason for doing this but I can't see it here so I'm just seeing a change that will possibly add some extra cost (if at least because file locations change again) and nothing more. When you try to reorganize things, it's often much more efficient to try to discuss it before proposing patches, because reorg patches are generally unreadable and take time for you to create and for others to review. Instead, just explaining what you think you can improve is faster for everyone, and others can chime in and propose alternate approaches (something which is very hard to do with a patch series). Thanks! Willy
Hi, Willy > > On Sat, Jul 08, 2023 at 11:26:42PM +0800, Zhangjin Wu wrote: > > Currently, the architecture specific arch.h has two parts, one is the > > syscall declarations for sys.h, another is the _start code definition > > for startup support. > > > > The coming crt.h will provide the startup support with a new common > > _start_c(), it will replace most of the assembly _start code and shrink > > the original _start code to be minimal, as a result, _start_c() and the > > left minimal _start code will work together to provide the startup > > support, therefore, the left _start code will be only required by crt.h. > > > > So, the syscall declarations part of arch.h can be split to sys_arch.h > > and the _start code part of arch.h can be split to crt_arch.h and then, > > they should only be included in sys.h and crt.h respectively. > > > > At the same time, the architecture specific arch-<ARCH>.h should be > > split to <ARCH>/crt.h and <ARCH>/sys.h. > > > > As a preparation, this creates the architecture specific directory and > > moves tools/include/nolibc/arch-<ARCH>.h to > > tools/include/nolibc/<ARCH>/arch.h. > > I'm sorry but I still don't understand what it *provides*. I'm reading > it as "we *can* do this so let's do it". But what is the specific > purpose of adding this extra directory structure ? It's really unclear > to me and worries me that it'll only result in complicating maintenance > by adding even more files, thus even more "include" lines and cross > dependencies. Willy, I was assuming you had a look at the discussion between Thomas and me, so, I didn't add the link to our discussion, it is more about the 'clarity' of code "include" [1]. I have proposed the idea in the discussion but got no response yet, so, sent this revision for more discussion, obviously, it is better to discuss more there and get more feedback from Thomas and you. The v0 included "crt.h" before "arch.h", Thomas suggested me include "crt.h" in arch_<ARCH>.h, just like the "compiler.h" did. His suggestion did inspire me to think about how to treat the relationship among crt.h, sys.h and arch.h. The idea behind is we have many directions to divide nolibc to different parts/modules: - one is arch specific (arch.h) and non-arch specific (the others) This method is used by us currently, It is very good to put all of the arch specific parts together to simplify (in the files to be added/maintained) the porting of a new architecture. But to be honest, It also confuse the modularity a little, for example, like sys.h, crt.h should be a core function/feature of nolibc, arch.h is not so. arch.h only provides the necessary minimal assembly "pieces". both sys.h and crt.h are not a sub modules of arch.h (although they have minimal arch specific code), so, like sys.h, crt.h should be included in the top-level headers, not in arch.h, reversely, the minimal arch specific should be included in crt.h. To do so and to avoid include the non-crt part, the split of arch.h is required, and therefore, the <ARCH>/ is created to put the divided <ARCH>/sys.h and <ARCH>/crt.h, otherwise, there will be many sys-<ARCH>.h and crt-<ARCH>.h in the top-level directory of nolibc. - another is the parallel functions/features (like crt.h, sys.h, stack protector ...) This is used by musl and glibc, before sending this proposal, I have taken a look at both of them, musl is simpler and clearer, we apply the similar method: musl: crt/crt1.c #include "crt_arch.h" /* arch/<ARCH>/crt_arch.h */ void _start_c(long *p) { int argc = p[0]; char **argv = (void *)(p+1); ... } src/internal/syscall.h: ##include "syscall_arch.h" /* arch/<ARCH>/syscall_arch.h */ ... glibc: (it is more complicated than musl) csu/libc-start.c, sysdeps/<ARCH>/start.S sysdeps/unix/sysv/linux/sysdep.h, sysdeps/unix/sysv/linux/<ARCH>/sysdep.h, With this method, the "crt_arch.h + crt.h" together provide the C RunTime (startup code, stack protector, environ and _auxv currently) function, the "sys_arch.h + sys.h" together provide the syscall definitions. The arch specific parts are hidden behind, and only require to include the crt_arch.h in crt.h and sys_arch.h in sys.h, no need to include the whole arch.h for all. As a summary, the core obvious reason here is, to this crt.h itself, it is ok for us to include crt.h in arch.h in code side, but reversely, I do prefer to include arch.h (and therefore the crt_arch.h) in crt.h, crt.h is the core function should be exported, arch.h is not, it only provide some low-level helpers for crt.h. If we treat sys.h as a core function and crt.h as a arch specific thing, it does confuse a little. This reorg may also help the similar future functions who require arch specific support, but of course, it does require to add/maintain more files for a new architecture, but it also allow to develop/debug at a smaller fineness. In current stage, include crt.h in arch.h is not that unacceptable, but if reorg is a better direction, why not do it currently, because we do have two functions (crt.h and sys.h) in <ARCH>/, if only one, it is not urgent ;-) Is this explanation better than before? welcome to discuss more ;-) Like musl, if required, another top-level arch/ may be required to put all of the <ARCH>/ directories together to clean up the top-level nolibc directory. > > Zhangjin, very often in your series, the justification for a change is > missing, instead it's only explaining what is being changed, and I must > confess that it makes it particularly difficult to figure the benefits. > I'm only seeing this as an opportunity for a change ("can be split"). > I could have missed something of course, but I can't figure what problem > it is trying to solve. Willy, thanks very much for pointing out this and so sorry, "commit message should tell why but not how" is in my mind, but sometimes, it may be lost especially when the change list are 'huge' (must improve). In reality, It is a little difficult to just explain it at the right dimension for this change, so I have wrotten several versions of the commit message for this change locally (and also for the other changes too), at last, I choose the one currently used. As explained in another reply, it is really hard to write a just ok commit message for every change, sometimes, the justification is 'obvious' to some develoers who have the background information or who have dicussed together, sometimes, it is not easy to just write precisely about the core justification, to improve this, here is the list I plan to do, hope it help the others too: - discuss more before send new patches, especially for 'new' or 'big' change - reword carefully about every change before sending patches (show benefits to let maintainer 'buy' (merge) them) - send the patches not frequently, keep the mind conscious, not like a "flood" > > As a general advice, I tend to remind people that when sending a patch > series, they should consider they're trying to sell it, so they must > emphasize the benefits of accepting the series for the maintainer(s). > Yeah, why maintainer want to 'buy' (merge) is the right direction. > You very likely have a good reason for doing this but I can't see it > here so I'm just seeing a change that will possibly add some extra > cost (if at least because file locations change again) and nothing > more. When you try to reorganize things, it's often much more > efficient to try to discuss it before proposing patches, because > reorg patches are generally unreadable and take time for you to > create and for others to review. Yes, reorg is really hard to do and also hard to review. > Instead, just explaining what you > think you can improve is faster for everyone, and others can chime in > and propose alternate approaches (something which is very hard to do > with a patch series). > Great suggestion, a ping on the discussion [1] would be better than just reorg it and send the patches. Thanks a lot. Best regards, Zhangjin --- [1]: https://lore.kernel.org/lkml/20230703145500.500460-1-falcon@tinylab.org/ > Thanks! > Willy
On 2023-07-10 15:23:40+0800, Zhangjin Wu wrote: > > On Sat, Jul 08, 2023 at 11:26:42PM +0800, Zhangjin Wu wrote: > [..] > > > As a preparation, this creates the architecture specific directory and > > > moves tools/include/nolibc/arch-<ARCH>.h to > > > tools/include/nolibc/<ARCH>/arch.h. > > > > I'm sorry but I still don't understand what it *provides*. I'm reading > > it as "we *can* do this so let's do it". But what is the specific > > purpose of adding this extra directory structure ? It's really unclear > > to me and worries me that it'll only result in complicating maintenance > > by adding even more files, thus even more "include" lines and cross > > dependencies. > > Willy, I was assuming you had a look at the discussion between Thomas > and me, so, I didn't add the link to our discussion, it is more about > the 'clarity' of code "include" [1]. > > I have proposed the idea in the discussion but got no response yet, so, > sent this revision for more discussion, obviously, it is better to > discuss more there and get more feedback from Thomas and you. To be honest I got overwhelmed at some point and instead of figuring out to which series' I already responded and which not I only responded to those where I had time to do so immediately. Keeping the amount of in-flight serieses lower would help this. > The v0 included "crt.h" before "arch.h", Thomas suggested me include > "crt.h" in arch_<ARCH>.h, just like the "compiler.h" did. His suggestion > did inspire me to think about how to treat the relationship among crt.h, > sys.h and arch.h. > > The idea behind is we have many directions to divide nolibc to different > parts/modules: > > - one is arch specific (arch.h) and non-arch specific (the others) > > This method is used by us currently, It is very good to put all of the > arch specific parts together to simplify (in the files to be > added/maintained) the porting of a new architecture. > > But to be honest, It also confuse the modularity a little, for > example, like sys.h, crt.h should be a core function/feature of > nolibc, arch.h is not so. arch.h only provides the necessary minimal > assembly "pieces". > > both sys.h and crt.h are not a sub modules of arch.h (although they > have minimal arch specific code), so, like sys.h, crt.h should be > included in the top-level headers, not in arch.h, reversely, the > minimal arch specific should be included in crt.h. To do so and to > avoid include the non-crt part, the split of arch.h is required, and > therefore, the <ARCH>/ is created to put the divided <ARCH>/sys.h and > <ARCH>/crt.h, otherwise, there will be many sys-<ARCH>.h and > crt-<ARCH>.h in the top-level directory of nolibc. > > - another is the parallel functions/features (like crt.h, sys.h, stack protector ...) > > This is used by musl and glibc, before sending this proposal, I have > taken a look at both of them, musl is simpler and clearer, we apply > the similar method: > > musl: > crt/crt1.c > #include "crt_arch.h" /* arch/<ARCH>/crt_arch.h */ In musl crt_arch.h seems to be used in different ways. So it makes sense to split it from syscall_arch.h. In nolibc there is no such distinction. And everything will end up in a global namespace anyways. > void _start_c(long *p) > { > int argc = p[0]; > char **argv = (void *)(p+1); > ... > } > > src/internal/syscall.h: > ##include "syscall_arch.h" /* arch/<ARCH>/syscall_arch.h */ > > ... > > glibc: (it is more complicated than musl) > > csu/libc-start.c, sysdeps/<ARCH>/start.S > > sysdeps/unix/sysv/linux/sysdep.h, sysdeps/unix/sysv/linux/<ARCH>/sysdep.h, > > > With this method, the "crt_arch.h + crt.h" together provide the C > RunTime (startup code, stack protector, environ and _auxv currently) > function, the "sys_arch.h + sys.h" together provide the syscall > definitions. The arch specific parts are hidden behind, and only > require to include the crt_arch.h in crt.h and sys_arch.h in sys.h, no > need to include the whole arch.h for all. > > As a summary, the core obvious reason here is, to this crt.h itself, it > is ok for us to include crt.h in arch.h in code side, but reversely, I > do prefer to include arch.h (and therefore the crt_arch.h) in crt.h, > crt.h is the core function should be exported, arch.h is not, it only > provide some low-level helpers for crt.h. If we treat sys.h as a core > function and crt.h as a arch specific thing, it does confuse a little. > This reorg may also help the similar future functions who require arch > specific support, but of course, it does require to add/maintain more > files for a new architecture, but it also allow to develop/debug at a > smaller fineness. > > In current stage, include crt.h in arch.h is not that unacceptable, but Why would it be more unacceptable in the future? > if reorg is a better direction, why not do it currently, because we do > have two functions (crt.h and sys.h) in <ARCH>/, if only one, it is not > urgent ;-) > Is this explanation better than before? welcome to discuss more ;-) Personally I'm not convinced :-) The arch-specific code in nolibc in mainline is currentl ~200 lines per arch. With this series in general it will be even less. If at some point there are many more architectures it may make sense to introduce an arch/ directory then. > Like musl, if required, another top-level arch/ may be required to put > all of the <ARCH>/ directories together to clean up the top-level nolibc > directory. At the moment in mainline there are 26 files in nolibc. That does not seem excessive, in fact it looks to be less than most other kernel directories. > > Zhangjin, very often in your series, the justification for a change is > > missing, instead it's only explaining what is being changed, and I must > > confess that it makes it particularly difficult to figure the benefits. > > I'm only seeing this as an opportunity for a change ("can be split"). > > I could have missed something of course, but I can't figure what problem > > it is trying to solve. > > Willy, thanks very much for pointing out this and so sorry, "commit > message should tell why but not how" is in my mind, but sometimes, it > may be lost especially when the change list are 'huge' (must improve). > > In reality, It is a little difficult to just explain it at the right > dimension for this change, so I have wrotten several versions of the > commit message for this change locally (and also for the other changes > too), at last, I choose the one currently used. > > As explained in another reply, it is really hard to write a just ok > commit message for every change, sometimes, the justification is > 'obvious' to some develoers who have the background information or who > have dicussed together, sometimes, it is not easy to just write > precisely about the core justification, to improve this, here is the > list I plan to do, hope it help the others too: > > - discuss more before send new patches, especially for 'new' or 'big' > change This sounds like the correct thing to do for reorganization patches. It should be very easy to describe and very annoying to actually do. > - reword carefully about every change before sending patches (show > benefits to let maintainer 'buy' (merge) them) > > - send the patches not frequently, keep the mind conscious, not like a > "flood" > [..]
Hi Thomas, Zhangjin, On Mon, Jul 10, 2023 at 05:51:39PM +0200, Thomas Weißschuh wrote: > On 2023-07-10 15:23:40+0800, Zhangjin Wu wrote: > > > On Sat, Jul 08, 2023 at 11:26:42PM +0800, Zhangjin Wu wrote: > > > [..] > > > > > As a preparation, this creates the architecture specific directory and > > > > moves tools/include/nolibc/arch-<ARCH>.h to > > > > tools/include/nolibc/<ARCH>/arch.h. > > > > > > I'm sorry but I still don't understand what it *provides*. I'm reading > > > it as "we *can* do this so let's do it". But what is the specific > > > purpose of adding this extra directory structure ? It's really unclear > > > to me and worries me that it'll only result in complicating maintenance > > > by adding even more files, thus even more "include" lines and cross > > > dependencies. > > > > Willy, I was assuming you had a look at the discussion between Thomas > > and me, so, I didn't add the link to our discussion, it is more about > > the 'clarity' of code "include" [1]. > > > > I have proposed the idea in the discussion but got no response yet, so, > > sent this revision for more discussion, obviously, it is better to > > discuss more there and get more feedback from Thomas and you. > > To be honest I got overwhelmed at some point and instead of figuring out > to which series' I already responded and which not I only responded to > those where I had time to do so immediately. > > Keeping the amount of in-flight serieses lower would help this. +1 on this. First it's difficult for me to assign contiguous time on the subject so I can't grasp all series at once, and I'm terribly bad at context-switching, which takes even more time and induces confusion. Less topics at once, more focused with less reviews of reorganizations will definitely help. > > The v0 included "crt.h" before "arch.h", Thomas suggested me include > > "crt.h" in arch_<ARCH>.h, just like the "compiler.h" did. His suggestion > > did inspire me to think about how to treat the relationship among crt.h, > > sys.h and arch.h. > > > > The idea behind is we have many directions to divide nolibc to different > > parts/modules: Again above I'm seeing an opportunity but no explanation of why this is needed. Thomas already mentioned that you're speaking about just trying to factor out a few tens of lines. I'm not seeing *why* we need to re-split everything yet again. > > - one is arch specific (arch.h) and non-arch specific (the others) > > > > This method is used by us currently, It is very good to put all of the > > arch specific parts together to simplify (in the files to be > > added/maintained) the porting of a new architecture. > > > > But to be honest, It also confuse the modularity a little, for > > example, like sys.h, crt.h should be a core function/feature of > > nolibc, arch.h is not so. arch.h only provides the necessary minimal > > assembly "pieces". But that's precisely the principle: keep arch-specific stuff as minimal as possible, keep most of the rest generic but easily overloadable if needed as we know that archs are not all 1:1 equivalent. > > both sys.h and crt.h are not a sub modules of arch.h (although they > > have minimal arch specific code), so, like sys.h, crt.h should be > > included in the top-level headers, not in arch.h, Why ? Keep in mind that these are only include files, to in the end, *all* of them are included. The ordering is the only thing that really matters. > > reversely, the > > minimal arch specific should be included in crt.h. To do so and to > > avoid include the non-crt part, the split of arch.h is required, and > > therefore, the <ARCH>/ is created to put the divided <ARCH>/sys.h and > > <ARCH>/crt.h, otherwise, there will be many sys-<ARCH>.h and > > crt-<ARCH>.h in the top-level directory of nolibc. Then doesn't it prove that you don't need that crt-<ARCH>.h and that instead it should just be in arch-<ARCH> like the rest of the same arch ? > > - another is the parallel functions/features (like crt.h, sys.h, stack protector ...) > > > > This is used by musl and glibc, before sending this proposal, I have > > taken a look at both of them, musl is simpler and clearer, we apply > > the similar method: > > > > musl: > > crt/crt1.c > > #include "crt_arch.h" /* arch/<ARCH>/crt_arch.h */ > > In musl crt_arch.h seems to be used in different ways. So it makes sense > to split it from syscall_arch.h. In nolibc there is no such distinction. > And everything will end up in a global namespace anyways. Exactly. Musl is musl and nolibc is nolibc. Musl is a regular libc in that it provides a .so that is built from many .c files. As such it's desirable to split along certain edges. nolibc contains no single C file. It's only meant to be included as-is in the user's C file. This changes a lot of things, even in terms of splitting. Also keep in mind that musl is a general-purpose libc, and that some distros are entirely built on it. nolibc doesn't have such goal nor expectation, the first user was a preinit code I wrote long ago, and the second one is rcutorture which contains a while() loop around gettimeofday() IIRC. We must not just blindly imitate other components' choices because they work, when we're dealing with different constraints. If ours are acceptable, no need to complicate everything. > > With this method, the "crt_arch.h + crt.h" together provide the C > > RunTime (startup code, stack protector, environ and _auxv currently) > > function, the "sys_arch.h + sys.h" together provide the syscall > > definitions. The arch specific parts are hidden behind, and only > > require to include the crt_arch.h in crt.h and sys_arch.h in sys.h, no > > need to include the whole arch.h for all. Everything is included all the time. *everything*. The more files we create, the more "#ifdef FOO_H" gets evaluated, and the more maintenance burden it adds. > > As a summary, the core obvious reason here is, to this crt.h itself, it > > is ok for us to include crt.h in arch.h in code side, but reversely, I > > do prefer to include arch.h (and therefore the crt_arch.h) in crt.h, > > crt.h is the core function should be exported, arch.h is not, it only > > provide some low-level helpers for crt.h. If we treat sys.h as a core > > function and crt.h as a arch specific thing, it does confuse a little. > > This reorg may also help the similar future functions who require arch > > specific support, but of course, it does require to add/maintain more > > files for a new architecture, but it also allow to develop/debug at a > > smaller fineness. > > > > In current stage, include crt.h in arch.h is not that unacceptable, but > > Why would it be more unacceptable in the future? > > > if reorg is a better direction, why not do it currently, because we do > > have two functions (crt.h and sys.h) in <ARCH>/, if only one, it is not > > urgent ;-) > > > Is this explanation better than before? welcome to discuss more ;-) > > Personally I'm not convinced :-) Neither am I. > The arch-specific code in nolibc in mainline is currentl ~200 lines per > arch. With this series in general it will be even less. > If at some point there are many more architectures it may make sense to > introduce an arch/ directory then. And even then I'm not convinced because the number of archs will remain low anyway. > > Like musl, if required, another top-level arch/ may be required to put > > all of the <ARCH>/ directories together to clean up the top-level nolibc > > directory. > > At the moment in mainline there are 26 files in nolibc. > That does not seem excessive, in fact it looks to be less than most > other kernel directories. Indeed :-) Note that it started with a single one! > > As explained in another reply, it is really hard to write a just ok > > commit message for every change, sometimes, the justification is > > 'obvious' to some develoers who have the background information or who > > have dicussed together, sometimes, Sometimes yes, but most of the series come with propositions to improve something. The commits making the major changes (and the cover letter) should justify why this is a desirable change, what it implies not to have it and what it may imply to have it, what possible alternatives were considered and dropped sometimes (e.g. when hesitating between two approaches), etc. Thanks, Willy
Hi Zhangjin, On Wed, Jul 12, 2023 at 12:38:30AM +0800, Zhangjin Wu wrote: > > > > both sys.h and crt.h are not a sub modules of arch.h (although they > > > > have minimal arch specific code), so, like sys.h, crt.h should be > > > > included in the top-level headers, not in arch.h, > > > > Why ? Keep in mind that these are only include files, to in the end, > > *all* of them are included. The ordering is the only thing that really > > matters. > > > > Yeah, I know this. The only thing confused me is the relationship among > crt.h, sys.h and arch.h, the include position should better reflects > their relationship, currently, we have mixed two "divide" methods > together, one is arch and non-arch, another is the parallel > functions/features. > > so, the only left question is where should we include crt.h in? Firstly, > I put it like sys.h (In my mind, it should be), If you two both agree to > put it in arch-<ARCH.h>, I will renew this series with it, this is > definitely a lighter way than the reorg method, I don't persist ;-) Then let's start this way. And if later we find a better organization *and* a good reason to change it, we can do it. Please also keep in mind that constantly renaming/moving inflicts extra pain to the stable team when backporting fixes if any. That's another reason for not moving code around often. > > > In musl crt_arch.h seems to be used in different ways. So it makes sense > > > to split it from syscall_arch.h. In nolibc there is no such distinction. > > > And everything will end up in a global namespace anyways. > > > > Exactly. Musl is musl and nolibc is nolibc. Musl is a regular libc in that > > it provides a .so that is built from many .c files. As such it's desirable > > to split along certain edges. nolibc contains no single C file. It's only > > meant to be included as-is in the user's C file. This changes a lot of > > things, even in terms of splitting. Also keep in mind that musl is a > > general-purpose libc, and that some distros are entirely built on it. > > nolibc doesn't have such goal nor expectation, the first user was a > > preinit code I wrote long ago, and the second one is rcutorture which > > contains a while() loop around gettimeofday() IIRC. We must not just > > blindly imitate other components' choices because they work, when we're > > dealing with different constraints. If ours are acceptable, no need to > > complicate everything. > > > > Willy, I know the difference between musl and nolibc, the musl code > referenced here is only used to clear my confusion about "the > relationship among crt.h, sys.h and arch.h" I mentiond above. OK, but even so, our includes are "private" and do not necessarily need to be organized like other projects. That doesn't mean that good ideas from other ones should not help us decide, of course, I just mean that it's normal to see differences. > BTW, I do think nolibc have more using scenes, I like it very much and > have used it in my own "Linux Lab" open source project [1] to let it > work as the minimal rootfs to speed up kernel features learning and > development, I do like the 'kernel-only deployments' feature behind > nolibc [2], although there is something like "Unikernel Linux" [3], but > that differs from a normal Linux system and is more complicated ;-) > > I'm even imaging using it with a pure-header shell and a pure-header gui > to let them further work together as a tiny rtos ;-) I'm not denying the possibilities for other use cases. I still have somewhere a small 1MB kernel image+rootfs that is sufficient to SSH into remotely or download and flash a firmware image over tftp/http/serial, so I see pretty well how this can be useful, and suspect this could eventually happen again. But doing incremental changes that don't seem to go in a particular direction is not much helpful and complicates participation from everyone. However of course, if someone comes and say "I'd like to use it in this or that environment but for this I need all that", it's easier to follow and try to steer in a direction that adapts smoothly to all users' needs. Another point, please also remember that we moved it into the kernel to help those who need to adjust their tests easily contribute the missing calls (like you did for a number of things). But I tend to think that as long as it's in the kernel, the activity should remain related to the kernel usage. If you have bigger plans for a much larger project, the kernel's process will slow you down and you'll experience some rejection for breaking certain principles, so in this case such work would better be done outside (after re-updating the original repo that I left rot for a while). > > > The arch-specific code in nolibc in mainline is currentl ~200 lines per > > > arch. With this series in general it will be even less. > > > If at some point there are many more architectures it may make sense to > > > introduce an arch/ directory then. > > > > And even then I'm not convinced because the number of archs will remain > > low anyway. > > > > We have 8 now, the maximum may be 'ls -d arch/*/ | wc -l', it is 22 > currently ;-) Yes but I'm not convinced that all of them will be ported there, for various reasons ranging from lack of interest or lack of use cases, to lack of maintainer time. > > > > Like musl, if required, another top-level arch/ may be required to put > > > > all of the <ARCH>/ directories together to clean up the top-level nolibc > > > > directory. > > > > > > At the moment in mainline there are 26 files in nolibc. > > > That does not seem excessive, in fact it looks to be less than most > > > other kernel directories. > > > > Indeed :-) Note that it started with a single one! > > Yeah, I learned the history, but I do think we will have more, as the > requirements become more and more ;-) Sure, but it grew fast thanks to being easy to understand and locate stuff. This should be seen as a feature I guess. Cheers, Willy
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile index 64d67b080744..ce21ace8210e 100644 --- a/tools/include/nolibc/Makefile +++ b/tools/include/nolibc/Makefile @@ -24,7 +24,7 @@ Q=@ endif nolibc_arch := $(patsubst arm64,aarch64,$(ARCH)) -arch_file := arch-$(nolibc_arch).h +arch_file := $(nolibc_arch)/arch.h all_files := \ compiler.h \ ctype.h \ @@ -70,11 +70,11 @@ headers: $(Q)cp $(all_files) $(OUTPUT)sysroot/include/ $(Q)if [ "$(ARCH)" = "x86" ]; then \ sed -e \ - 's,^#ifndef _NOLIBC_ARCH_X86_64_H,#if !defined(_NOLIBC_ARCH_X86_64_H) \&\& defined(__x86_64__),' \ - arch-x86_64.h; \ + 's,^#ifndef _NOLIBC_X86_64_ARCH_H,#if !defined(_NOLIBC_X86_64_ARCH_H) \&\& defined(__x86_64__),' \ + x86_64/arch.h; \ sed -e \ - 's,^#ifndef _NOLIBC_ARCH_I386_H,#if !defined(_NOLIBC_ARCH_I386_H) \&\& !defined(__x86_64__),' \ - arch-i386.h; \ + 's,^#ifndef _NOLIBC_I386_ARCH_H,#if !defined(_NOLIBC_I386_ARCH_H) \&\& !defined(__x86_64__),' \ + i386/arch.h; \ elif [ -e "$(arch_file)" ]; then \ cat $(arch_file); \ else \ diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/aarch64/arch.h similarity index 99% rename from tools/include/nolibc/arch-aarch64.h rename to tools/include/nolibc/aarch64/arch.h index 6227b77a4a09..7d38da13c72b 100644 --- a/tools/include/nolibc/arch-aarch64.h +++ b/tools/include/nolibc/aarch64/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_AARCH64_H -#define _NOLIBC_ARCH_AARCH64_H +#ifndef _NOLIBC_AARCH64_ARCH_H +#define _NOLIBC_AARCH64_ARCH_H #include "compiler.h" @@ -201,4 +201,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr ); __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_AARCH64_H */ +#endif /* _NOLIBC_AARCH64_ARCH_H */ diff --git a/tools/include/nolibc/arch.h b/tools/include/nolibc/arch.h index 82b43935650f..f98616f5b219 100644 --- a/tools/include/nolibc/arch.h +++ b/tools/include/nolibc/arch.h @@ -16,21 +16,21 @@ #define _NOLIBC_ARCH_H #if defined(__x86_64__) -#include "arch-x86_64.h" +#include "x86_64/arch.h" #elif defined(__i386__) || defined(__i486__) || defined(__i586__) || defined(__i686__) -#include "arch-i386.h" +#include "i386/arch.h" #elif defined(__ARM_EABI__) -#include "arch-arm.h" +#include "arm/arch.h" #elif defined(__aarch64__) -#include "arch-aarch64.h" +#include "aarch64/arch.h" #elif defined(__mips__) && defined(_ABIO32) -#include "arch-mips.h" +#include "mips/arch.h" #elif defined(__riscv) -#include "arch-riscv.h" +#include "riscv/arch.h" #elif defined(__s390x__) -#include "arch-s390.h" +#include "s390/arch.h" #elif defined(__loongarch__) -#include "arch-loongarch.h" +#include "loongarch/arch.h" #endif #endif /* _NOLIBC_ARCH_H */ diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arm/arch.h similarity index 99% rename from tools/include/nolibc/arch-arm.h rename to tools/include/nolibc/arm/arch.h index 4d4887a5f04b..473d2c000740 100644 --- a/tools/include/nolibc/arch-arm.h +++ b/tools/include/nolibc/arm/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_ARM_H -#define _NOLIBC_ARCH_ARM_H +#ifndef _NOLIBC_ARM_ARCH_H +#define _NOLIBC_ARM_ARCH_H #include "compiler.h" @@ -267,4 +267,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_ARM_H */ +#endif /* _NOLIBC_ARM_ARCH_H */ diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/i386/arch.h similarity index 99% rename from tools/include/nolibc/arch-i386.h rename to tools/include/nolibc/i386/arch.h index 4c6b7c04e2e7..66052742763e 100644 --- a/tools/include/nolibc/arch-i386.h +++ b/tools/include/nolibc/i386/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_I386_H -#define _NOLIBC_ARCH_I386_H +#ifndef _NOLIBC_I386_ARCH_H +#define _NOLIBC_I386_ARCH_H #include "compiler.h" @@ -221,4 +221,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_I386_H */ +#endif /* _NOLIBC_I386_ARCH_H */ diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/loongarch/arch.h similarity index 99% rename from tools/include/nolibc/arch-loongarch.h rename to tools/include/nolibc/loongarch/arch.h index 8aa7724fe38e..63fee1e8f4d9 100644 --- a/tools/include/nolibc/arch-loongarch.h +++ b/tools/include/nolibc/loongarch/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2023 Loongson Technology Corporation Limited */ -#ifndef _NOLIBC_ARCH_LOONGARCH_H -#define _NOLIBC_ARCH_LOONGARCH_H +#ifndef _NOLIBC_LOONGARCH_ARCH_H +#define _NOLIBC_LOONGARCH_ARCH_H #include "compiler.h" @@ -197,4 +197,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_LOONGARCH_H */ +#endif /* _NOLIBC_LOONGARCH_ARCH_H */ diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/mips/arch.h similarity index 99% rename from tools/include/nolibc/arch-mips.h rename to tools/include/nolibc/mips/arch.h index a2bfdf57b957..1581b721b714 100644 --- a/tools/include/nolibc/arch-mips.h +++ b/tools/include/nolibc/mips/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_MIPS_H -#define _NOLIBC_ARCH_MIPS_H +#ifndef _NOLIBC_MIPS_ARCH_H +#define _NOLIBC_MIPS_ARCH_H #include "compiler.h" @@ -250,4 +250,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_MIPS_H */ +#endif /* _NOLIBC_MIPS_ARCH_H */ diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/riscv/arch.h similarity index 99% rename from tools/include/nolibc/arch-riscv.h rename to tools/include/nolibc/riscv/arch.h index cd958b2f4b1b..de68759f5959 100644 --- a/tools/include/nolibc/arch-riscv.h +++ b/tools/include/nolibc/riscv/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_RISCV_H -#define _NOLIBC_ARCH_RISCV_H +#ifndef _NOLIBC_RISCV_ARCH_H +#define _NOLIBC_RISCV_ARCH_H #include "compiler.h" @@ -214,4 +214,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_RISCV_H */ +#endif /* _NOLIBC_RISCV_ARCH_H */ diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/s390/arch.h similarity index 98% rename from tools/include/nolibc/arch-s390.h rename to tools/include/nolibc/s390/arch.h index a644ecd361c0..a7b512e81234 100644 --- a/tools/include/nolibc/arch-s390.h +++ b/tools/include/nolibc/s390/arch.h @@ -3,8 +3,8 @@ * s390 specific definitions for NOLIBC */ -#ifndef _NOLIBC_ARCH_S390_H -#define _NOLIBC_ARCH_S390_H +#ifndef _NOLIBC_S390_ARCH_H +#define _NOLIBC_S390_ARCH_H #include <asm/signal.h> #include <asm/unistd.h> @@ -234,4 +234,4 @@ pid_t sys_fork(void) } #define sys_fork sys_fork -#endif /* _NOLIBC_ARCH_S390_H */ +#endif /* _NOLIBC_S390_ARCH_H */ diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/x86_64/arch.h similarity index 99% rename from tools/include/nolibc/arch-x86_64.h rename to tools/include/nolibc/x86_64/arch.h index e69113742a99..602791c3461a 100644 --- a/tools/include/nolibc/arch-x86_64.h +++ b/tools/include/nolibc/x86_64/arch.h @@ -4,8 +4,8 @@ * Copyright (C) 2017-2022 Willy Tarreau <w@1wt.eu> */ -#ifndef _NOLIBC_ARCH_X86_64_H -#define _NOLIBC_ARCH_X86_64_H +#ifndef _NOLIBC_X86_64_ARCH_H +#define _NOLIBC_X86_64_ARCH_H #include "compiler.h" @@ -217,4 +217,4 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr __builtin_unreachable(); } -#endif /* _NOLIBC_ARCH_X86_64_H */ +#endif /* _NOLIBC_X86_64_ARCH_H */
Currently, the architecture specific arch.h has two parts, one is the syscall declarations for sys.h, another is the _start code definition for startup support. The coming crt.h will provide the startup support with a new common _start_c(), it will replace most of the assembly _start code and shrink the original _start code to be minimal, as a result, _start_c() and the left minimal _start code will work together to provide the startup support, therefore, the left _start code will be only required by crt.h. So, the syscall declarations part of arch.h can be split to sys_arch.h and the _start code part of arch.h can be split to crt_arch.h and then, they should only be included in sys.h and crt.h respectively. At the same time, the architecture specific arch-<ARCH>.h should be split to <ARCH>/crt.h and <ARCH>/sys.h. As a preparation, this creates the architecture specific directory and moves tools/include/nolibc/arch-<ARCH>.h to tools/include/nolibc/<ARCH>/arch.h. Signed-off-by: Zhangjin Wu <falcon@tinylab.org> --- tools/include/nolibc/Makefile | 10 +++++----- .../nolibc/{arch-aarch64.h => aarch64/arch.h} | 6 +++--- tools/include/nolibc/arch.h | 16 ++++++++-------- tools/include/nolibc/{arch-arm.h => arm/arch.h} | 6 +++--- .../include/nolibc/{arch-i386.h => i386/arch.h} | 6 +++--- .../{arch-loongarch.h => loongarch/arch.h} | 6 +++--- .../include/nolibc/{arch-mips.h => mips/arch.h} | 6 +++--- .../nolibc/{arch-riscv.h => riscv/arch.h} | 6 +++--- .../include/nolibc/{arch-s390.h => s390/arch.h} | 6 +++--- .../nolibc/{arch-x86_64.h => x86_64/arch.h} | 6 +++--- 10 files changed, 37 insertions(+), 37 deletions(-) rename tools/include/nolibc/{arch-aarch64.h => aarch64/arch.h} (99%) rename tools/include/nolibc/{arch-arm.h => arm/arch.h} (99%) rename tools/include/nolibc/{arch-i386.h => i386/arch.h} (99%) rename tools/include/nolibc/{arch-loongarch.h => loongarch/arch.h} (99%) rename tools/include/nolibc/{arch-mips.h => mips/arch.h} (99%) rename tools/include/nolibc/{arch-riscv.h => riscv/arch.h} (99%) rename tools/include/nolibc/{arch-s390.h => s390/arch.h} (98%) rename tools/include/nolibc/{arch-x86_64.h => x86_64/arch.h} (99%)