From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Alistair Francis <alistair.francis@wdc.com>, libc-alpha@sourceware.org
Cc: stepan@golosunov.pp.ru, alistair23@gmail.com
Subject: Re: [PATCH v9 0/2] Support semctl_syscall() for __TIMESIZE==64
Date: Mon, 29 Jun 2020 17:51:33 -0300 [thread overview]
Message-ID: <35e8928c-e76d-5af7-72d4-db9f4e4bd94e@linaro.org> (raw)
In-Reply-To: <20200602204105.43866-1-alistair.francis@wdc.com>
On 02/06/2020 17:41, Alistair Francis wrote:
> This series supports the semctl calls on a system with __TIMESIZE==64
> and __WORDSIZE==32 while not breaking current architectures. This is a
> step towards full y2038 support, but does not get us there yet.
>
> See: https://sourceware.org/pipermail/libc-alpha/2020-May/113774.html
> for more details on what is still required.
>
> This series adds a new __semid_ds32 that is passed to the kernel
> (as part of a union) when running on 32-bit systems. If we are doing an
> IPC_STAT/SEM_STAT command then the 32-bit sem_{c,o}time{_high} values
> are combined to create a 64-bit value.
I think this patch still does not go in to the direction it was discussed
in last iteration. This patch just handles the case of 32-bit
architectures with only 64-bit time_t support, where to fully cover all
possible scenarios we will need to handle the 32-bit architectures where
it would be possible to select the time_t ABI (i386 for instance).
To move this support forward I implemented the idea we discussed in a
personal branch [1]. Instead of adding the the '__semid_ds32' I added
two new structures:
1. kernel_semid64_ds: used internally only on 32-bit architectures
to issue the syscall. As for __semid_ds32, a handful architectures
(hppa, i386, mips, powerpc32, sparc32) requires specific
implementation due its specific kernel ABI.
2. semid_ds64: this is only for __TIMESIZE != 64 to used along with
the 64-bit semctl. It is different than the kernel one because
the exported 64-bit time_t might require different alignment
depending of the architecture ABI.
So the resulting implementation does:
1. For 64-bit architectures it assumes semid_ds already contains
64-bit time_t fields it will result in just the __semctl symbol
using the __semctl64 code:
static int
semctl_syscall (int semid, int semnum, int cmd, union semun arg)
{
[...]
}
int
semctl ([...])
{
union semun arg64 = { 0 };
[...]
switch (cmd)
{
[...]
arg64 = va_arg (ap, union semun);
[...]
}
union semun arg = arg64;
int ret = semctl_syscall (semid, semnum, cmd, arg);
[...]
}
Basically the semid_ds argument is passed as-is to the kernel
interface.
2. For 32-bit architectures with default 64-bit time_t (newer ABIs
such riscv32 or arc), it will also result in only one symbol
but with the required high/low handling:
static int
semctl_syscall (int semid, int semnum, int cmd, union ksemun64 arg)
{
[...]
}
int
semctl ([...])
{
union semun arg64 = { 0 };
[...]
switch (cmd)
{
[...]
arg64 = va_arg (ap, union semun64);
[...]
}
struct kernel_semid64_ds ksemid;
union ksemun64 ksemun = semun64_to_ksemun64 (cmd, arg64, &ksemid);
union ksemun64 arg = ksemun;
int ret = semctl_syscall (semid, semnum, cmd, arg);
[...]
switch (cmd)
{
case IPC_STAT:
case SEM_STAT:
case SEM_STAT_ANY:
[...]
ksemid64_to_semid64 (arg.buf, arg64.buf);
}
[...]
}
It might be possible to optimize it further to avoid the
kernel_semid64_ds to semun transformation if the exported glibc ABI
for the architectures matches the expected kernel ABI, but the
implementation is already complex enough and don't think this should
be a hotspot in any case.
3. Finally for 32-bit architecture with both 32-bit and 64-bit time_t
support we follow the already set way to provide one symbol with
64-bit time_t support and implement the 32-bit time_t support on
basis of the 64-bit one:
static int
semctl_syscall (int semid, int semnum, int cmd, union ksemun64 arg)
{
[...]
}
int
__semctl64 ([...])
{
union semun64 arg64 = { 0 };
[...]
switch (cmd)
{
[...]
arg64 = va_arg (ap, union semun64);
[...]
}
struct kernel_semid64_ds ksemid;
union ksemun64 ksemun = semun64_to_ksemun64 (cmd, arg64, &ksemid);
union ksemun64 arg = ksemun;
int ret = semctl_syscall (semid, semnum, cmd, arg);
[...]
switch (cmd)
{
case IPC_STAT:
case SEM_STAT:
case SEM_STAT_ANY:
[...]
ksemid64_to_semid64 (arg.buf, arg64.buf);
}
[...]
}
int
__semctl (int semid, int semnum, int cmd, ...)
{
union semun arg = { 0 };
[...]
switch (cmd)
{
[...]
arg = va_arg (ap, union semun);
[...]
}
struct __semid64_ds semid64;
union semun64 arg64 = semun_to_semun64 (cmd, arg, &semid64);
int ret = __semctl64 (semid, semnum, cmd, arg64);
switch (cmd)
{
case IPC_STAT:
case SEM_STAT:
case SEM_STAT_ANY:
semid64_ds_to_semid_ds (arg.buf, arg64.buf);
}
}
The default 32-bit symbol will allocate and copy the semid_ds
over multiple buffers, but this should be deprecated in favor
of the __semctl64 anyway.
I did some sniff tests on i686, arm, mips, and powerpc where this
code should change the way the function is implemented. Could you
also check on riscv32 since currently there is no 32-bit architecture
with default 64-bit time_t? If it ok for riscv32 I can send it
upstream.
[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/semctl-y2038
prev parent reply other threads:[~2020-06-29 20:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-02 20:41 Alistair Francis
2020-06-02 20:41 ` [PATCH v9 1/2] sysv: linux: Define the __semid_ds32 struct Alistair Francis
2020-06-10 20:44 ` Alistair Francis
2020-06-15 22:35 ` Alistair Francis
2020-06-25 23:35 ` Alistair Francis
2020-06-02 20:41 ` [PATCH v9 2/2] sysv: linux: Pass 64-bit version of semctl syscall Alistair Francis
2020-06-29 20:51 ` Adhemerval Zanella [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=35e8928c-e76d-5af7-72d4-db9f4e4bd94e@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=libc-alpha@sourceware.org \
--cc=stepan@golosunov.pp.ru \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).