From: "André Almeida" <andrealmeid@collabora.com> To: Alistair Francis <alistair23@gmail.com> Cc: Alistair Francis <alistair.francis@opensource.wdc.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org>, Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>, linux-perf-users@vger.kernel.org, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Mark Rutland <mark.rutland@arm.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Davidlohr Bueso <dave@stgolabs.net>, Darren Hart <dvhart@infradead.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, Atish Patra <atish.patra@wdc.com>, Arnd Bergmann <arnd@arndb.de>, Alistair Francis <alistair.francis@wdc.com> Subject: Re: [PATCH v3 2/2] perf bench: Add support for 32-bit systems with 64-bit time_t Date: Sun, 26 Sep 2021 18:32:57 -0300 [thread overview] Message-ID: <a65dfe31-a355-8cf8-99d8-70ddf23c5384@collabora.com> (raw) In-Reply-To: <CAKmqyKM+VN-KST9-VMULZMC=2sNbjH2wiE-CZ1WRfVFj3WmpdQ@mail.gmail.com> Às 01:34 de 24/09/21, Alistair Francis escreveu: > On Tue, Sep 21, 2021 at 8:47 AM André Almeida <andrealmeid@collabora.com> wrote: >> >> Hi Alistair, >> >> Às 03:10 de 17/09/21, Alistair Francis escreveu: >>> From: Alistair Francis <alistair.francis@wdc.com> >>> >>> Some 32-bit architectures (such are 32-bit RISC-V) only have a 64-bit >>> time_t and as such don't have the SYS_futex syscall. This patch will >>> allow us to use the SYS_futex_time64 syscall on those platforms. >>> >> >> Thanks for your patch! However, I don't think that any futex operation >> at perf has timeout. Do you plan to implement a test that use it? Or the >> idea is to get this ready for it in case someone want to do so in the >> future? > > I don't have plans to implement any new tests (although I'm happy to > add one if need be). > > My goal was just to get this to build for RISC-V 32-bit. The timeout > was already exposed by the old futex macro, so I was just following > that. > I see, thanks for working on that. >> >> >> Also, I faced a similar problem with the new futex2 syscalls, that >> supports exclusively 64bit timespec. But I took a different approach: I >> called __NR_clock_gettime64 for 32bit architectures so it wouldn't >> require to convert the struct: >> >> #if defined(__i386__) || __TIMESIZE == 32 >> # define NR_gettime64 __NR_clock_gettime64 >> #else >> # define NR_gettime64 __NR_clock_gettime >> #endif >> >> struct timespec64 { >> long long tv_sec; /* seconds */ >> long long tv_nsec; /* nanoseconds */ >> }; >> >> int gettime64(clock_t clockid, struct timespec64 *tv) >> { >> return syscall(NR_gettime64, clockid, tv); >> } >> >> Then we can just use &timeout at __NR_futex_time64 for 32bit arch and at >> __NR_futex for 64bit arch. > > So the idea is to use 64-bit time_t everywhere and only work on 5.1+ kernels. > > If that's the favoured approach I can convert this series to your idea. > Yes, this is what I think it will be the best approach. I believe the code will be less complex, it's more future proof (it's ready for y2038) and when glibc supports time64, we can make this code even simpler using `-D__USE_TIME_BITS64` to compile it. Thanks again for working on that! > Alistair > >> >> This might be a simpler solution to the problem that you are facing but >> I'm not entirely sure. Also, futex's selftests do use the timeout >> argument and I think that they also won't compile in 32-bit RISC-V, so >> maybe we can start from there so we can actually test the timeout >> argument and check if it's working. >> >> Thanks, >> André
WARNING: multiple messages have this Message-ID (diff)
From: "André Almeida" <andrealmeid@collabora.com> To: Alistair Francis <alistair23@gmail.com> Cc: Alistair Francis <alistair.francis@opensource.wdc.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org>, Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>, linux-perf-users@vger.kernel.org, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Mark Rutland <mark.rutland@arm.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Davidlohr Bueso <dave@stgolabs.net>, Darren Hart <dvhart@infradead.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, Atish Patra <atish.patra@wdc.com>, Arnd Bergmann <arnd@arndb.de>, Alistair Francis <alistair.francis@wdc.com> Subject: Re: [PATCH v3 2/2] perf bench: Add support for 32-bit systems with 64-bit time_t Date: Sun, 26 Sep 2021 18:32:57 -0300 [thread overview] Message-ID: <a65dfe31-a355-8cf8-99d8-70ddf23c5384@collabora.com> (raw) In-Reply-To: <CAKmqyKM+VN-KST9-VMULZMC=2sNbjH2wiE-CZ1WRfVFj3WmpdQ@mail.gmail.com> Às 01:34 de 24/09/21, Alistair Francis escreveu: > On Tue, Sep 21, 2021 at 8:47 AM André Almeida <andrealmeid@collabora.com> wrote: >> >> Hi Alistair, >> >> Às 03:10 de 17/09/21, Alistair Francis escreveu: >>> From: Alistair Francis <alistair.francis@wdc.com> >>> >>> Some 32-bit architectures (such are 32-bit RISC-V) only have a 64-bit >>> time_t and as such don't have the SYS_futex syscall. This patch will >>> allow us to use the SYS_futex_time64 syscall on those platforms. >>> >> >> Thanks for your patch! However, I don't think that any futex operation >> at perf has timeout. Do you plan to implement a test that use it? Or the >> idea is to get this ready for it in case someone want to do so in the >> future? > > I don't have plans to implement any new tests (although I'm happy to > add one if need be). > > My goal was just to get this to build for RISC-V 32-bit. The timeout > was already exposed by the old futex macro, so I was just following > that. > I see, thanks for working on that. >> >> >> Also, I faced a similar problem with the new futex2 syscalls, that >> supports exclusively 64bit timespec. But I took a different approach: I >> called __NR_clock_gettime64 for 32bit architectures so it wouldn't >> require to convert the struct: >> >> #if defined(__i386__) || __TIMESIZE == 32 >> # define NR_gettime64 __NR_clock_gettime64 >> #else >> # define NR_gettime64 __NR_clock_gettime >> #endif >> >> struct timespec64 { >> long long tv_sec; /* seconds */ >> long long tv_nsec; /* nanoseconds */ >> }; >> >> int gettime64(clock_t clockid, struct timespec64 *tv) >> { >> return syscall(NR_gettime64, clockid, tv); >> } >> >> Then we can just use &timeout at __NR_futex_time64 for 32bit arch and at >> __NR_futex for 64bit arch. > > So the idea is to use 64-bit time_t everywhere and only work on 5.1+ kernels. > > If that's the favoured approach I can convert this series to your idea. > Yes, this is what I think it will be the best approach. I believe the code will be less complex, it's more future proof (it's ready for y2038) and when glibc supports time64, we can make this code even simpler using `-D__USE_TIME_BITS64` to compile it. Thanks again for working on that! > Alistair > >> >> This might be a simpler solution to the problem that you are facing but >> I'm not entirely sure. Also, futex's selftests do use the timeout >> argument and I think that they also won't compile in 32-bit RISC-V, so >> maybe we can start from there so we can actually test the timeout >> argument and check if it's working. >> >> Thanks, >> André _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-09-26 21:33 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-17 6:10 [PATCH v3 1/2] perf benchmark: Call the futex syscall from a function Alistair Francis 2021-09-17 6:10 ` Alistair Francis 2021-09-17 6:10 ` [PATCH v3 2/2] perf bench: Add support for 32-bit systems with 64-bit time_t Alistair Francis 2021-09-17 6:10 ` Alistair Francis 2021-09-17 7:33 ` Arnd Bergmann 2021-09-17 7:33 ` Arnd Bergmann 2021-09-17 18:33 ` Davidlohr Bueso 2021-09-17 18:33 ` Davidlohr Bueso 2021-09-20 22:47 ` André Almeida 2021-09-20 22:47 ` André Almeida 2021-09-21 8:08 ` Arnd Bergmann 2021-09-21 8:08 ` Arnd Bergmann 2021-09-21 23:06 ` André Almeida 2021-09-21 23:06 ` André Almeida 2021-09-22 11:26 ` Arnd Bergmann 2021-09-22 11:26 ` Arnd Bergmann 2021-09-22 11:27 ` Arnd Bergmann 2021-09-22 11:27 ` Arnd Bergmann 2021-09-24 4:34 ` Alistair Francis 2021-09-24 4:34 ` Alistair Francis 2021-09-24 4:34 ` Alistair Francis 2021-09-24 4:34 ` Alistair Francis 2021-09-26 21:32 ` André Almeida [this message] 2021-09-26 21:32 ` André Almeida 2021-09-17 18:21 ` [PATCH v3 1/2] perf benchmark: Call the futex syscall from a function Davidlohr Bueso 2021-09-17 18:21 ` Davidlohr Bueso
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=a65dfe31-a355-8cf8-99d8-70ddf23c5384@collabora.com \ --to=andrealmeid@collabora.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=alistair.francis@opensource.wdc.com \ --cc=alistair.francis@wdc.com \ --cc=alistair23@gmail.com \ --cc=arnd@arndb.de \ --cc=atish.patra@wdc.com \ --cc=dave@stgolabs.net \ --cc=dvhart@infradead.org \ --cc=jolsa@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-perf-users@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.