2023-06-03 09:22:09

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 0/3] nolibc: add part2 of support for rv32

Hi, Willy

This is the v3 part2 of support for rv32, differs from the v2 part2 [1],
we only fix up compile issues in this patchset.

With the v3 generic part1 [2] and this patchset, we can compile nolibc
for rv32 now.

This is based on the idea of suggestions from Arnd [3], instead of
'#error' on the unsupported syscall on a target platform, a 'return
-ENOSYS' allow us to compile it at first and then allow we fix up the
test failures reported by nolibc-test one by one.

The first two patches fix up all of the compile failures with '-ENOSYS'
(and '#ifdef' if required):

tools/nolibc: fix up #error compile failures with -ENOSYS
tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS

The last one enables rv32 compile support:

selftests/nolibc: riscv: customize makefile for rv32

The above compile support patch here is only for test currently, as
Thomas suggested, for a full rv32 support, it should wait for the left
parts.

Welcome your feedbacks, will wait for enough discussion on this patchset
and then send the left parts one by one to fix up the test failures
about waitid, llseek and time64 syscalls: ppoll_time64, clock_gettime64,
pselect6_time64.

So, I do recommend to apply this patchset, it allows us to send the left
parts independently, otherwise, all of them should be sent out for
review together. with this patchset, the rv32 users may be able to use
nolibc although some syscalls still missing :-)

Or at least we apply the first two, so, I can manually cherry-pick the
compile support patch to do my local test, and the other platform
developer may also benefit from them.

I'm cleaning up the left parts, but still require some time, I plan to
split them to such parts:

* part3: waitid, prepared, will send out later
* part4: llseek, prepared, will send out later
* part5: time64 syscalls, ppoll_time64 ok, will finish them next week
(It is a little hard to split them)

Best regards,
Zhangjin
---

[1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
[2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
[3]: https://lore.kernel.org/linux-riscv/[email protected]/

Zhangjin Wu (3):
tools/nolibc: fix up #error compile failures with -ENOSYS
tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
selftests/nolibc: riscv: customize makefile for rv32

tools/include/nolibc/sys.h | 38 ++++++++++++++++---------
tools/testing/selftests/nolibc/Makefile | 11 +++++--
2 files changed, 34 insertions(+), 15 deletions(-)

--
2.25.1



2023-06-03 09:23:21

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

Compiling nolibc for rv32 got such errors:

In file included from nolibc/sysroot/riscv/include/nolibc.h:99,
from nolibc/sysroot/riscv/include/errno.h:26,
from nolibc/sysroot/riscv/include/stdio.h:14,
from tools/testing/selftests/nolibc/nolibc-test.c:12:
nolibc/sysroot/riscv/include/sys.h:946:2: error: #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll()
946 | #error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll()
| ^~~~~
nolibc/sysroot/riscv/include/sys.h:1062:2: error: #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()
1062 | #error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()

If a syscall is not supported by a target platform, 'return -ENOSYS' is
better than '#error', which lets the other syscalls work as-is and
allows developers to fix up the test failures reported by nolibc-test
one by one later.

This converts all of the '#error' to 'return -ENOSYS', so, all of the
'#error' failures are fixed.

Suggested-by: Arnd Bergmann <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/include/nolibc/sys.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 856249a11890..78c86f124335 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode)
#elif defined(__NR_chmod)
return my_syscall2(__NR_chmod, path, mode);
#else
-#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement sys_chmod()
+ return -ENOSYS;
#endif
}

@@ -153,7 +153,7 @@ int sys_chown(const char *path, uid_t owner, gid_t group)
#elif defined(__NR_chown)
return my_syscall3(__NR_chown, path, owner, group);
#else
-#error Neither __NR_fchownat nor __NR_chown defined, cannot implement sys_chown()
+ return -ENOSYS;
#endif
}

@@ -251,7 +251,7 @@ int sys_dup2(int old, int new)
#elif defined(__NR_dup2)
return my_syscall2(__NR_dup2, old, new);
#else
-#error Neither __NR_dup3 nor __NR_dup2 defined, cannot implement sys_dup2()
+ return -ENOSYS;
#endif
}

@@ -351,7 +351,7 @@ pid_t sys_fork(void)
#elif defined(__NR_fork)
return my_syscall0(__NR_fork);
#else
-#error Neither __NR_clone nor __NR_fork defined, cannot implement sys_fork()
+ return -ENOSYS;
#endif
}
#endif
@@ -648,7 +648,7 @@ int sys_link(const char *old, const char *new)
#elif defined(__NR_link)
return my_syscall2(__NR_link, old, new);
#else
-#error Neither __NR_linkat nor __NR_link defined, cannot implement sys_link()
+ return -ENOSYS;
#endif
}

@@ -700,7 +700,7 @@ int sys_mkdir(const char *path, mode_t mode)
#elif defined(__NR_mkdir)
return my_syscall2(__NR_mkdir, path, mode);
#else
-#error Neither __NR_mkdirat nor __NR_mkdir defined, cannot implement sys_mkdir()
+ return -ENOSYS;
#endif
}

@@ -729,7 +729,7 @@ long sys_mknod(const char *path, mode_t mode, dev_t dev)
#elif defined(__NR_mknod)
return my_syscall3(__NR_mknod, path, mode, dev);
#else
-#error Neither __NR_mknodat nor __NR_mknod defined, cannot implement sys_mknod()
+ return -ENOSYS;
#endif
}

@@ -848,7 +848,7 @@ int sys_open(const char *path, int flags, mode_t mode)
#elif defined(__NR_open)
return my_syscall3(__NR_open, path, flags, mode);
#else
-#error Neither __NR_openat nor __NR_open defined, cannot implement sys_open()
+ return -ENOSYS;
#endif
}

@@ -943,7 +943,7 @@ int sys_poll(struct pollfd *fds, int nfds, int timeout)
#elif defined(__NR_poll)
return my_syscall3(__NR_poll, fds, nfds, timeout);
#else
-#error Neither __NR_ppoll nor __NR_poll defined, cannot implement sys_poll()
+ return -ENOSYS;
#endif
}

@@ -1059,7 +1059,7 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
#endif
return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
#else
-#error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select()
+ return -ENOSYS;
#endif
}

@@ -1196,7 +1196,7 @@ int sys_stat(const char *path, struct stat *buf)
#elif defined(__NR_stat)
ret = my_syscall2(__NR_stat, path, &stat);
#else
-#error Neither __NR_newfstatat nor __NR_stat defined, cannot implement sys_stat()
+ return -ENOSYS;
#endif
buf->st_dev = stat.st_dev;
buf->st_ino = stat.st_ino;
@@ -1243,7 +1243,7 @@ int sys_symlink(const char *old, const char *new)
#elif defined(__NR_symlink)
return my_syscall2(__NR_symlink, old, new);
#else
-#error Neither __NR_symlinkat nor __NR_symlink defined, cannot implement sys_symlink()
+ return -ENOSYS;
#endif
}

@@ -1312,7 +1312,7 @@ int sys_unlink(const char *path)
#elif defined(__NR_unlink)
return my_syscall1(__NR_unlink, path);
#else
-#error Neither __NR_unlinkat nor __NR_unlink defined, cannot implement sys_unlink()
+ return -ENOSYS;
#endif
}

--
2.25.1


2023-06-06 05:01:26

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 0/3] nolibc: add part2 of support for rv32

Hi, Arnd

Because this patchset is a 'big' change derived from the idea of suggestion
from you [3], I do very welcome your feedback about this change, just like
Thomas suggested, this requires more discussion before Willy plan to determine
merge it or not.

The first two convert all compile failures to a return of -ENOSYS, if you do
like it, welcome your Reviewed-by. These two are required by the coming new
time64 syscalls for rv32, because they depends on how we cope with the
unsupported syscalls, returning -ENOSYS is really better than simply fail the
compiling.

Hi, Thomas, If you are happy with them, welcome your Reviewed-by too, thanks.
If the first two are ok, then, I can focus on preparing the left time64
patchsets.

The third one is not that urgent, because some important syscalls are
still missing for rv32. It is added here only for compile test.

Thanks,
Zhangjin

> Hi, Willy
>
> This is the v3 part2 of support for rv32, differs from the v2 part2 [1],
> we only fix up compile issues in this patchset.
>
> With the v3 generic part1 [2] and this patchset, we can compile nolibc
> for rv32 now.
>
> This is based on the idea of suggestions from Arnd [3], instead of
> '#error' on the unsupported syscall on a target platform, a 'return
> -ENOSYS' allow us to compile it at first and then allow we fix up the
> test failures reported by nolibc-test one by one.
>
> The first two patches fix up all of the compile failures with '-ENOSYS'
> (and '#ifdef' if required):
>
> tools/nolibc: fix up #error compile failures with -ENOSYS
> tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
>
> The last one enables rv32 compile support:
>
> selftests/nolibc: riscv: customize makefile for rv32
>
> The above compile support patch here is only for test currently, as
> Thomas suggested, for a full rv32 support, it should wait for the left
> parts.
>
> Welcome your feedbacks, will wait for enough discussion on this patchset
> and then send the left parts one by one to fix up the test failures
> about waitid, llseek and time64 syscalls: ppoll_time64, clock_gettime64,
> pselect6_time64.
>
> So, I do recommend to apply this patchset, it allows us to send the left
> parts independently, otherwise, all of them should be sent out for
> review together. with this patchset, the rv32 users may be able to use
> nolibc although some syscalls still missing :-)
>
> Or at least we apply the first two, so, I can manually cherry-pick the
> compile support patch to do my local test, and the other platform
> developer may also benefit from them.
>
> I'm cleaning up the left parts, but still require some time, I plan to
> split them to such parts:
>
> * part3: waitid, prepared, will send out later
> * part4: llseek, prepared, will send out later
> * part5: time64 syscalls, ppoll_time64 ok, will finish them next week
> (It is a little hard to split them)
>
> Best regards,
> Zhangjin
> ---
>
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> [2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> [3]: https://lore.kernel.org/linux-riscv/[email protected]/
>
> Zhangjin Wu (3):
> tools/nolibc: fix up #error compile failures with -ENOSYS
> tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
> selftests/nolibc: riscv: customize makefile for rv32
>
> tools/include/nolibc/sys.h | 38 ++++++++++++++++---------
> tools/testing/selftests/nolibc/Makefile | 11 +++++--
> 2 files changed, 34 insertions(+), 15 deletions(-)

2023-06-06 05:13:14

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] nolibc: add part2 of support for rv32

Hi Zhangjin,

On Tue, Jun 06, 2023 at 12:25:35PM +0800, Zhangjin Wu wrote:
> The first two convert all compile failures to a return of -ENOSYS, if you do
> like it, welcome your Reviewed-by. These two are required by the coming new
> time64 syscalls for rv32, because they depends on how we cope with the
> unsupported syscalls, returning -ENOSYS is really better than simply fail the
> compiling.

I had a look now and I can sya that I like this. Initially the supported
syscalls were so restricted that it was not even imaginable to accept to
build without any of them, but now that we're completing the list, some
of them are less critical and I don't see why we'd fail to build just
because one is missing. So yeah, a big +1 for -ENOSYS.

> The third one is not that urgent, because some important syscalls are
> still missing for rv32. It is added here only for compile test.

I personally have no opinion on this one. I can't judge whether it will
make things easier or more complicated at this point. It seems to me
that for now it's just avoiding one extra line at the expense of some
$(if) on several lines. Maybe it could help add more such archs, or
maybe it can make them more complicated to debug, I don't know. I'm
interested in others' opinions as well.

Thanks,
Willy

2023-06-06 07:16:20

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] nolibc: add part2 of support for rv32

Willy, Thomas, Arnd

> Hi Zhangjin,
>
> On Tue, Jun 06, 2023 at 12:25:35PM +0800, Zhangjin Wu wrote:
> > The first two convert all compile failures to a return of -ENOSYS, if you do
> > like it, welcome your Reviewed-by. These two are required by the coming new
> > time64 syscalls for rv32, because they depends on how we cope with the
> > unsupported syscalls, returning -ENOSYS is really better than simply fail the
> > compiling.
>
> I had a look now and I can sya that I like this. Initially the supported
> syscalls were so restricted that it was not even imaginable to accept to
> build without any of them, but now that we're completing the list, some
> of them are less critical and I don't see why we'd fail to build just
> because one is missing. So yeah, a big +1 for -ENOSYS.
>

Cool, I will prepare the new patchsets on them, welcome your new branch
with both of them, of course, still weclome Reviewed-by from Arnd and Thomas.

> > The third one is not that urgent, because some important syscalls are
> > still missing for rv32. It is added here only for compile test.
>
> I personally have no opinion on this one. I can't judge whether it will
> make things easier or more complicated at this point. It seems to me
> that for now it's just avoiding one extra line at the expense of some
> $(if) on several lines. Maybe it could help add more such archs, or
> maybe it can make them more complicated to debug, I don't know. I'm
> interested in others' opinions as well.

As I explained why we did it in current way in this reply [1], Thomas had no
more questions on it, so I think Thomas was happy with it now and I got his
only left suggestion is that may be this patch should be applied after the
missing 64bit syscalls being added for there are several important test
failures currently, for me, it is ok before or after.

Thomas, welcome your Reviewed-by on the makefile patch itself If you are really
happy with it now, thanks very much ;-)

Willy, I will send the v2 syscalls helpers (also required by the coming 64bit
syscalls) and some other patches (mainly for test with faster kernel build)
about selftests/nolibc later, because we have not enough time for v6.5 test,
so, I suggest we can create new branch for v6.6 and my new patchsets will be
for v6.6.

Best regards,
Zhangjin

---

[1]: https://lore.kernel.org/linux-riscv/[email protected]/

>
> Thanks,
> Willy

2023-06-06 07:17:34

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] nolibc: add part2 of support for rv32

On Tue, Jun 06, 2023 at 02:34:21PM +0800, Zhangjin Wu wrote:
> Willy, Thomas, Arnd
>
> > Hi Zhangjin,
> >
> > On Tue, Jun 06, 2023 at 12:25:35PM +0800, Zhangjin Wu wrote:
> > > The first two convert all compile failures to a return of -ENOSYS, if you do
> > > like it, welcome your Reviewed-by. These two are required by the coming new
> > > time64 syscalls for rv32, because they depends on how we cope with the
> > > unsupported syscalls, returning -ENOSYS is really better than simply fail the
> > > compiling.
> >
> > I had a look now and I can sya that I like this. Initially the supported
> > syscalls were so restricted that it was not even imaginable to accept to
> > build without any of them, but now that we're completing the list, some
> > of them are less critical and I don't see why we'd fail to build just
> > because one is missing. So yeah, a big +1 for -ENOSYS.
> >
>
> Cool, I will prepare the new patchsets on them, welcome your new branch
> with both of them, of course, still weclome Reviewed-by from Arnd and Thomas.

I don't even think a new branch is needed, I can take them as-is it seems.

> > > The third one is not that urgent, because some important syscalls are
> > > still missing for rv32. It is added here only for compile test.
> >
> > I personally have no opinion on this one. I can't judge whether it will
> > make things easier or more complicated at this point. It seems to me
> > that for now it's just avoiding one extra line at the expense of some
> > $(if) on several lines. Maybe it could help add more such archs, or
> > maybe it can make them more complicated to debug, I don't know. I'm
> > interested in others' opinions as well.
>
> As I explained why we did it in current way in this reply [1], Thomas had no
> more questions on it, so I think Thomas was happy with it now and I got his
> only left suggestion is that may be this patch should be applied after the
> missing 64bit syscalls being added for there are several important test
> failures currently, for me, it is ok before or after.
>
> Thomas, welcome your Reviewed-by on the makefile patch itself If you are really
> happy with it now, thanks very much ;-)
>
> Willy, I will send the v2 syscalls helpers (also required by the coming 64bit
> syscalls) and some other patches (mainly for test with faster kernel build)
> about selftests/nolibc later, because we have not enough time for v6.5 test,
> so, I suggest we can create new branch for v6.6 and my new patchsets will be
> for v6.6.

Agreed, thank you!
Willy

2023-06-06 07:42:02

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
>
> Suggested-by: Arnd Bergmann <[email protected]>
> Link:
> https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Zhangjin Wu <[email protected]>
> ---
> tools/include/nolibc/sys.h | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> index 856249a11890..78c86f124335 100644
> --- a/tools/include/nolibc/sys.h
> +++ b/tools/include/nolibc/sys.h
> @@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode)
> #elif defined(__NR_chmod)
> return my_syscall2(__NR_chmod, path, mode);
> #else
> -#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement
> sys_chmod()
> + return -ENOSYS;
> #endif
> }

I think the most logical would be to have each syscall (chmod,
fchmodat, ...) have its own function that returns -ENOSYS if
that is not defined, and have the logic that decides which one
to use as a separate function.

This patch is a step in that direction though, so I think that's
totally fine.

Arnd

2023-06-07 05:36:01

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
> >
> > Suggested-by: Arnd Bergmann <[email protected]>
> > Link:
> > https://lore.kernel.org/linux-riscv/[email protected]/
> > Signed-off-by: Zhangjin Wu <[email protected]>
> > ---
> > tools/include/nolibc/sys.h | 26 +++++++++++++-------------
> > 1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > index 856249a11890..78c86f124335 100644
> > --- a/tools/include/nolibc/sys.h
> > +++ b/tools/include/nolibc/sys.h
> > @@ -124,7 +124,7 @@ int sys_chmod(const char *path, mode_t mode)
> > #elif defined(__NR_chmod)
> > return my_syscall2(__NR_chmod, path, mode);
> > #else
> > -#error Neither __NR_fchmodat nor __NR_chmod defined, cannot implement
> > sys_chmod()
> > + return -ENOSYS;
> > #endif
> > }
>
> I think the most logical would be to have each syscall (chmod,
> fchmodat, ...) have its own function that returns -ENOSYS if
> that is not defined, and have the logic that decides which one
> to use as a separate function.
>

Yeah, agreed, we can clean up them one by one, If split them to their own
syscalls, I have two questions (for Arnd, and Willy too):

1. do we need to add the corresponding library routines at the same time?

Use llseek() as an example, there will be llseek() and lsee64(). If off_t
would be converted to 64bit, then, they can be simply added as follows:

#define lseek64 lseek
#define llseek lseek

Or something like this:

static __attribute__((unused))
loff_t lseek(int fd, loff_t offset, int whence)
{
return lseek(fd, offset, whence);
}

static __attribute__((unused))
off64_t lseek(int fd, off64_t offset, int whence)
{
return lseek(fd, offset, whence);
}

This one aligns with the other already added library routines.

Which one do you like more?

2. If so, how to cope with the new types when add the library routines?

Still use the above llseek() as an example, If not use '#define' method,
We may need to declare loff_t and off64_t in std.h too:

#define off64_t off_t
#define loff_t off_t

Or align with the other new types, use 'typedef' instead of '#define'.

And further, use poll() as an example, in its manpage [1], there may be some
new types, such as 'nfds_t', but 'int' is used in tools/include/nolibc/sys.h
currently, do we need to add nfds_t?

The 'idtypes_t' and 'id_t' types used by waitid() [2] is similar, both
of them can simply use the 'int' type.

The above two questions are important to the coming patches, it may determine
how I should tune the new llseek() and waitid() syscalls and their library
routines. very welcome your suggestions.

> This patch is a step in that direction though, so I think that's
> totally fine.

Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
send v4 now ;-)

Best regards,
Zhangjin

---
[1]: https://linux.die.net/man/2/poll
[2]: https://linux.die.net/man/2/waitid

>
> Arnd

2023-06-07 08:50:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
>> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
>
> Yeah, agreed, we can clean up them one by one, If split them to their own
> syscalls, I have two questions (for Arnd, and Willy too):
>
> 1. do we need to add the corresponding library routines at the same time?
>
> Use llseek() as an example, there will be llseek() and lsee64(). If off_t
> would be converted to 64bit, then, they can be simply added as follows:
>
> #define lseek64 lseek
> #define llseek lseek
>
> Or something like this:
>
> static __attribute__((unused))
> loff_t lseek(int fd, loff_t offset, int whence)
> {
> return lseek(fd, offset, whence);
> }
>
> static __attribute__((unused))
> off64_t lseek(int fd, off64_t offset, int whence)
> {
> return lseek(fd, offset, whence);
> }
>
> This one aligns with the other already added library routines.
>
> Which one do you like more?

lseek() is probably not a good example, as the llseek and lseek64
library functions are both nonstandard, and I'd suggest leaving them
out of nolibc altogether.

Are there any examples of functions where we actually want mulitple
versions?

> 2. If so, how to cope with the new types when add the library routines?
>
> Still use the above llseek() as an example, If not use '#define' method,
> We may need to declare loff_t and off64_t in std.h too:
>
> #define off64_t off_t
> #define loff_t off_t
>
> Or align with the other new types, use 'typedef' instead of '#define'.

If we do want to include the explicit off64_t interfaces from glibc,
I'd suggest doing it the same way as musl:
https://git.musl-libc.org/cgit/musl/tree/include/unistd.h#n201

>
>> This patch is a step in that direction though, so I think that's
>> totally fine.
>
> Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
> send v4 now ;-)

Yes, please do.

Arnd

2023-06-07 10:23:44

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

> On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
> >> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
> >
> > Yeah, agreed, we can clean up them one by one, If split them to their own
> > syscalls, I have two questions (for Arnd, and Willy too):
> >
> > 1. do we need to add the corresponding library routines at the same time?
> >
> > Use llseek() as an example, there will be llseek() and lsee64(). If off_t
> > would be converted to 64bit, then, they can be simply added as follows:
> >
> > #define lseek64 lseek
> > #define llseek lseek
> >
> > Or something like this:
> >
> > static __attribute__((unused))
> > loff_t lseek(int fd, loff_t offset, int whence)
> > {
> > return lseek(fd, offset, whence);
> > }
> >
> > static __attribute__((unused))
> > off64_t lseek(int fd, off64_t offset, int whence)
> > {
> > return lseek(fd, offset, whence);
> > }
> >
> > This one aligns with the other already added library routines.
> >
> > Which one do you like more?
>
> lseek() is probably not a good example, as the llseek and lseek64
> library functions are both nonstandard, and I'd suggest leaving them
> out of nolibc altogether.
>

Ok, agree, as the 64bit version of lseek may be enough for nolibc, if a target
application really require, they can add the alias themselves.

> Are there any examples of functions where we actually want mulitple
> versions?
>

For example, the following ones are related to the syscalls being added,
all of them have similar library routines in current sys.h:

* waitid, https://linux.die.net/man/2/waitid
* ppoll, https://linux.die.net/man/2/ppoll
* pselect, https://linux.die.net/man/2/pselect6
* clock_gettime, https://linux.die.net/man/2/clock_gettime

The similar routines are put in right side:

* waitid --> waitpid, wait, wait4
* ppoll --> poll
* pselect --> select
* clock_gettime --> gettimeofday

For the clock_gettime, it may also let us think about if we need to add
its friends (clock_getres, clock_settime) together.

> > 2. If so, how to cope with the new types when add the library routines?
> >
> > Still use the above llseek() as an example, If not use '#define' method,
> > We may need to declare loff_t and off64_t in std.h too:
> >
> > #define off64_t off_t
> > #define loff_t off_t
> >
> > Or align with the other new types, use 'typedef' instead of '#define'.
>
> If we do want to include the explicit off64_t interfaces from glibc,
> I'd suggest doing it the same way as musl:
> https://git.musl-libc.org/cgit/musl/tree/include/unistd.h#n201
>

Thanks.

> >
> >> This patch is a step in that direction though, so I think that's
> >> totally fine.
> >
> > Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
> > send v4 now ;-)
>
> Yes, please do.
>

Thanks very much, just added the Reviewed-by lines in v4 and have
already sent v4 out, welcome your review on the revision of the 3rd one,
it is almost consistent with the original Makefile now.

Best regards,
Zhangjin

> Arnd

2023-06-07 10:24:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

On Wed, Jun 7, 2023, at 11:46, Zhangjin Wu wrote:
>> On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
>
> Ok, agree, as the 64bit version of lseek may be enough for nolibc, if a target
> application really require, they can add the alias themselves.
>
>> Are there any examples of functions where we actually want mulitple
>> versions?
>>
>
> For example, the following ones are related to the syscalls being added,
> all of them have similar library routines in current sys.h:
>
> * waitid, https://linux.die.net/man/2/waitid
> * ppoll, https://linux.die.net/man/2/ppoll
> * pselect, https://linux.die.net/man/2/pselect6
> * clock_gettime, https://linux.die.net/man/2/clock_gettime
>
> The similar routines are put in right side:
>
> * waitid --> waitpid, wait, wait4
> * ppoll --> poll
> * pselect --> select
> * clock_gettime --> gettimeofday

Ok, I think these are all useful to have in both versions.

All four of these examples are old enough that I think it's
sufficient just expose them to userspace as the bare system
calls, and have the older library calls be implemented using
them without a fallback to the native syscalls of the same
name on architectures that have both, newer architectures
would only have the latest version anyway.

> For the clock_gettime, it may also let us think about if we need to add
> its friends (clock_getres, clock_settime) together.

Yes, I think that makes sense. We also need clock_settime()
to implement settimeofday() on rv32.

Ideally, I'd love to extend the tooling around system calls
in the kernel so we can automatically generate the low-level
wrapper functions from syscall.tbl, but this needs a lot of
other work that you should not need to depend on for what
you are doing right now.

Arnd

2023-06-07 13:35:59

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS

> On Wed, Jun 7, 2023, at 11:46, Zhangjin Wu wrote:
> >> On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
> >
> > Ok, agree, as the 64bit version of lseek may be enough for nolibc, if a target
> > application really require, they can add the alias themselves.
> >
> >> Are there any examples of functions where we actually want mulitple
> >> versions?
> >>
> >
> > For example, the following ones are related to the syscalls being added,
> > all of them have similar library routines in current sys.h:
> >
> > * waitid, https://linux.die.net/man/2/waitid
> > * ppoll, https://linux.die.net/man/2/ppoll
> > * pselect, https://linux.die.net/man/2/pselect6
> > * clock_gettime, https://linux.die.net/man/2/clock_gettime
> >
> > The similar routines are put in right side:
> >
> > * waitid --> waitpid, wait, wait4
> > * ppoll --> poll
> > * pselect --> select
> > * clock_gettime --> gettimeofday
>
> Ok, I think these are all useful to have in both versions.
>
> All four of these examples are old enough that I think it's
> sufficient just expose them to userspace as the bare system
> calls, and have the older library calls be implemented using
> them without a fallback to the native syscalls of the same
> name on architectures that have both, newer architectures
> would only have the latest version anyway.
>

Ok, Thanks, I have already added parts of them, will send waitid and
64bit lseek at first.

> > For the clock_gettime, it may also let us think about if we need to add
> > its friends (clock_getres, clock_settime) together.
>
> Yes, I think that makes sense. We also need clock_settime()
> to implement settimeofday() on rv32.
>

Ok.

> Ideally, I'd love to extend the tooling around system calls
> in the kernel so we can automatically generate the low-level
> wrapper functions from syscall.tbl,

That's cool.

BTW, I did something on dead syscall elimination [1] (DSE, RFC
patchset), a v1 has been prepared locally, but not sent out yet.

It also requires to work with the syscall.tbl or the generic
include/uapi/asm-generic/unistd.h, welcome your feedback on the RFC
patchset [1] and you should be the right reviewer of the coming v1 ;-)

The left issue of RFC version is finding a way to not KEEP the exception
entries (in ld script) added by get_user/put_user() if the corresponding
syscalls are not really used, such KEEPs of exception entries reverts
the ownership from "syscalls -> get_user/put_user" to "get_user/put_user
-> syscalls" and blocks the gc'ing of the sections of such syscalls.

In the coming v1, I used a script trick to drop the wrongly KEPT
exception entries to allow drop all of the unused syscalls at last. Will
clean up them asap. But it is a little slow and looks ugly, it is only
for a further demo of the possibility.

In v2 of DSE, I'm wondering whether it is possible to drop all of the
manually added KEEP operations from ld scripts and use some conditional
attributes (for the sections added by get_user/put_user) to build the
'used' references from "syscalls" to "sections created by
get_user/put_user", this may need support from gcc and ld, welcome your
suggestions too, thanks.

And that RFC patchset added a patch to record the used 'syscalls' in
nolibc automatically ;-)

[1]: https://lore.kernel.org/linux-riscv/[email protected]/
[2]: https://reviews.llvm.org/D96838

> but this needs a lot of
> other work that you should not need to depend on for what
> you are doing right now.

Ok, welcome to share any progress.

Thanks,
Zhangjin

>
> Arnd