From: "Jérémie Galarneau via lttng-dev" <lttng-dev@lists.lttng.org>
To: Alistair Francis <alistair23@gmail.com>
Cc: Alistair Francis <alistair.francis@opensource.wdc.com>,
lttng-dev@lists.lttng.org
Subject: Re: [lttng-dev] [PATCH lttng-tools 2/2] Tests: select_poll_epoll: Add support for _time64
Date: Thu, 13 Oct 2022 08:08:04 +0100 [thread overview]
Message-ID: <Y0e5VMVzJ1HBGe3M@efficios.com> (raw)
In-Reply-To: <CAKmqyKM=EN0NnKPcysDdUk+yPR6D=69G8pLTkrUYKHGj_47cpw@mail.gmail.com>
On Thu, Oct 13, 2022 at 10:12:48AM +1000, Alistair Francis wrote:
> On Wed, Oct 12, 2022 at 11:19 PM Jérémie Galarneau
> <jeremie.galarneau@efficios.com> wrote:
> >
> > On Fri, Oct 07, 2022 at 10:39:18AM +1000, Alistair Francis via lttng-dev wrote:
> >
> > Hi Alistair,
> >
> > The first patch is good, I'll merge it in master.
> > Some comments on this patch follow.
>
> Thanks!
>
> >
> > > From: Alistair Francis <alistair.francis@wdc.com>
> > >
> > > Add support for the 64-bit time_t syscalls SYS_ppoll_time64
> > > and SYS_pselect6_time64.
> > >
> > > These are the syscalls that exist 32-bit platforms since the 5.1 kernel.
> > > 32-bit platforms with a 64-bit time_t only have these and don't have the
> > > original syscalls (such as 32-bit RISC-V).
> > >
> > > Fixes: https://github.com/lttng/lttng-tools/pull/162
> > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > > ---
> > > tests/regression/kernel/select_poll_epoll.cpp | 53 +++++++++++++++++++
> > > 1 file changed, 53 insertions(+)
> > >
> > > diff --git a/tests/regression/kernel/select_poll_epoll.cpp b/tests/regression/kernel/select_poll_epoll.cpp
> > > index c0b688217..4a6d394f4 100644
> > > --- a/tests/regression/kernel/select_poll_epoll.cpp
> > > +++ b/tests/regression/kernel/select_poll_epoll.cpp
> > > @@ -5,6 +5,7 @@
> > > *
> > > */
> > >
> > > +#include <errno.h>
> > > #include <fcntl.h>
> > > #include <limits.h>
> > > #include <poll.h>
> > > @@ -456,8 +457,22 @@ void ppoll_fds_buffer_overflow(
> > > ufds[0].fd = wait_fd;
> > > ufds[0].events = POLLIN|POLLPRI;
> > >
> > > +#ifdef SYS_ppoll_time64
> > > + /*
> > > + * As there is no timeout value, we don't convert to/from
> > > + * 64/32-bit time_t.
> > > + */
> > > + ret = syscall(SYS_ppoll_time64, ufds, 100, NULL, NULL);
> > > + if (ret == 0 || errno != ENOSYS) {
> > > + goto ppoll_fds_buffer_overflow_done;
> > > + }
> > > +#endif
> > > +
> >
> > This results in the following warning when building for an architecture
> > that doesn't have SYS_ppoll_time64 defined:
> >
> > label ‘ppoll_fds_buffer_overflow_done’ defined but not used [-Wunused-label]
> > ppoll_fds_buffer_overflow_done:
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > select_poll_epoll.cpp: In function ‘void ppoll_fds_ulong_max(FILE*)’:
>
> Argh, I'll fix this
>
> >
> >
> > Also, it is my understanding that both syscalls can be available on some
> > platforms. In that case, it would make sense to add them as separate
> > tests and skip tests that target non-existant syscalls.
>
> So all 32-bit platforms since the 5.1 (or 5.4?) kernel have both syscalls.
>
> From my understanding the original syscalls will be removed on 32-bit
> platforms at some point (before 2038) and there will only be *_time64
> variants.
>
> If you want I can copy the tests to test both syscall types, but I
> don't think that's necessary.
>
My fear is that on those platforms the test will pass if the kernel tracer
succeeds in tracing any of the two syscalls.
I think it will be easier to simply have separate tests than validate the
two invocations independently in the trace.
Thanks!
Jérémie
> Alistair
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
prev parent reply other threads:[~2022-10-13 7:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-07 0:39 [lttng-dev] [PATCH lttng-tools 1/2] README: Update the Userspace RCU requirements Alistair Francis via lttng-dev
2022-10-07 0:39 ` [lttng-dev] [PATCH lttng-tools 2/2] Tests: select_poll_epoll: Add support for _time64 Alistair Francis via lttng-dev
2022-10-12 13:19 ` Jérémie Galarneau via lttng-dev
2022-10-13 0:12 ` Alistair Francis via lttng-dev
2022-10-13 7:08 ` Jérémie Galarneau via lttng-dev [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y0e5VMVzJ1HBGe3M@efficios.com \
--to=lttng-dev@lists.lttng.org \
--cc=alistair.francis@opensource.wdc.com \
--cc=alistair23@gmail.com \
--cc=jeremie.galarneau@efficios.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).