[v2,1/6] sysv/linux: Rename alpha functions to be alpha specific
Commit Message
These functions are alpha specifc, rename them to be clear.
Let's also rename the header file from tv32-compat.h to
alpha-tv32-compat.h. This is to avoid conflicts with the one we will
introduce later.
---
.../alpha/{tv32-compat.h => alpha-tv32-compat.h} | 16 ++++++++--------
sysdeps/unix/sysv/linux/alpha/osf_adjtime.c | 10 +++++-----
sysdeps/unix/sysv/linux/alpha/osf_getitimer.c | 6 +++---
sysdeps/unix/sysv/linux/alpha/osf_getrusage.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_gettimeofday.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 10 +++++-----
sysdeps/unix/sysv/linux/alpha/osf_settimeofday.c | 4 ++--
sysdeps/unix/sysv/linux/alpha/osf_utimes.c | 6 +++---
sysdeps/unix/sysv/linux/alpha/osf_wait4.c | 4 ++--
9 files changed, 32 insertions(+), 32 deletions(-)
rename sysdeps/unix/sysv/linux/alpha/{tv32-compat.h => alpha-tv32-compat.h} (88%)
Comments
On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> These functions are alpha specifc, rename them to be clear.
These functions were meant to be generic. I put the file in
s/u/s/l/alpha because it was only *used* on Alpha, at the time.
What's stopping you from using these functions on all architectures?
zw
On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>
> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > These functions are alpha specifc, rename them to be clear.
>
> These functions were meant to be generic. I put the file in
> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>
> What's stopping you from using these functions on all architectures?
The Alpha functions use 32-bit versions of the timeval struct for a
64-bit architecture. For all other architectures we want to use the
word size version (regardless of time_t size) for the timeval struct.
That's the main difference.
If Alpha didn't have this requirement we could then just remove the
Alpha specific overrides after this series and use all of the new
generic y2038 infrastructure.
Alistair
>
> zw
On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > These functions are alpha specifc, rename them to be clear.
> >
> > These functions were meant to be generic. I put the file in
> > s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >
> > What's stopping you from using these functions on all architectures?
>
> The Alpha functions use 32-bit versions of the timeval struct for a
> 64-bit architecture. For all other architectures we want to use the
> word size version (regardless of time_t size) for the timeval struct.
> That's the main difference.
I'm sorry, I haven't been following the discussion all that closely.
Why is it necessary to use a 32-bit timeval struct (with 32-bit
time_t, I presume) *only* on Alpha? Is there no way we can smooth
over this difference so that, as you say,
> If Alpha didn't have this requirement we could then just remove the
> Alpha specific overrides after this series and use all of the new
> generic y2038 infrastructure.
Also, even if we really are stuck with an Alpha-specific struct
timeval32, I don't see why that affects the definitions of the
conversion functions?
zw
On 10/02/2020 18:35, Zack Weinberg wrote:
> On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
>> On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>>>
>>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
>>> <alistair.francis@wdc.com> wrote:
>>>>
>>>> These functions are alpha specifc, rename them to be clear.
>>>
>>> These functions were meant to be generic. I put the file in
>>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>>>
>>> What's stopping you from using these functions on all architectures?
>>
>> The Alpha functions use 32-bit versions of the timeval struct for a
>> 64-bit architecture. For all other architectures we want to use the
>> word size version (regardless of time_t size) for the timeval struct.
>> That's the main difference.
>
> I'm sorry, I haven't been following the discussion all that closely.
> Why is it necessary to use a 32-bit timeval struct (with 32-bit
> time_t, I presume) *only* on Alpha? Is there no way we can smooth
> over this difference so that, as you say,
Alpha is an outlier here, it is the only 64-bit architecture that did
the 32 bit to 64 bit transition some time ago. All the generic code
assumes that 64-bit architectures always have 64-bit time_t
(__ASSUME_TIME64_SYSCALLS).
And the old interface is not exported and *only* meant to be used in
compat symbols, different than the current way of handling y2038
prof code (where the user might still define the ABI with _TIME_SIZE).
That's why I think alpha compat code should be compartmentalized on
alpha sysdep folder.
>
>> If Alpha didn't have this requirement we could then just remove the
>> Alpha specific overrides after this series and use all of the new
>> generic y2038 infrastructure.
>
> Also, even if we really are stuck with an Alpha-specific struct
> timeval32, I don't see why that affects the definitions of the
> conversion functions?
>
> zw
>
On Mon, Feb 10, 2020 at 1:53 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/02/2020 18:35, Zack Weinberg wrote:
> > On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
> >> On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> >>> <alistair.francis@wdc.com> wrote:
> >>>>
> >>>> These functions are alpha specifc, rename them to be clear.
> >>>
> >>> These functions were meant to be generic. I put the file in
> >>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >>>
> >>> What's stopping you from using these functions on all architectures?
> >>
> >> The Alpha functions use 32-bit versions of the timeval struct for a
> >> 64-bit architecture. For all other architectures we want to use the
> >> word size version (regardless of time_t size) for the timeval struct.
> >> That's the main difference.
> >
> > I'm sorry, I haven't been following the discussion all that closely.
> > Why is it necessary to use a 32-bit timeval struct (with 32-bit
> > time_t, I presume) *only* on Alpha? Is there no way we can smooth
> > over this difference so that, as you say,
>
> Alpha is an outlier here, it is the only 64-bit architecture that did
> the 32 bit to 64 bit transition some time ago. All the generic code
> assumes that 64-bit architectures always have 64-bit time_t
> (__ASSUME_TIME64_SYSCALLS).
>
> And the old interface is not exported and *only* meant to be used in
> compat symbols, different than the current way of handling y2038
> prof code (where the user might still define the ABI with _TIME_SIZE).
>
> That's why I think alpha compat code should be compartmentalized on
> alpha sysdep folder.
Agreed. That is the goal with this patch.
This patch moves and renames the Alpha functions to make sure there is
no conflict with any of the new work.
Alistair
>
> >
> >> If Alpha didn't have this requirement we could then just remove the
> >> Alpha specific overrides after this series and use all of the new
> >> generic y2038 infrastructure.
> >
> > Also, even if we really are stuck with an Alpha-specific struct
> > timeval32, I don't see why that affects the definitions of the
> > conversion functions?
> >
> > zw
> >
On Mon, 10 Feb 2020 09:43:20 -0800
Alistair Francis <alistair.francis@wdc.com> wrote:
> These functions are alpha specifc, rename them to be clear.
>
> Let's also rename the header file from tv32-compat.h to
> alpha-tv32-compat.h. This is to avoid conflicts with the one we will
> introduce later.
Reviewed-by: Lukasz Majewski <lukma@denx.de>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Mon, Feb 10, 2020 at 4:53 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 10/02/2020 18:35, Zack Weinberg wrote:
> > On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
> >> On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
> >>> <alistair.francis@wdc.com> wrote:
> >>>>
> >>>> These functions are alpha specifc, rename them to be clear.
> >>>
> >>> These functions were meant to be generic. I put the file in
> >>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
> >>>
> >>> What's stopping you from using these functions on all architectures?
> >>
> >> The Alpha functions use 32-bit versions of the timeval struct for a
> >> 64-bit architecture. For all other architectures we want to use the
> >> word size version (regardless of time_t size) for the timeval struct.
> >> That's the main difference.
> >
> > I'm sorry, I haven't been following the discussion all that closely.
> > Why is it necessary to use a 32-bit timeval struct (with 32-bit
> > time_t, I presume) *only* on Alpha? Is there no way we can smooth
> > over this difference so that, as you say,
>
> Alpha is an outlier here, it is the only 64-bit architecture that did
> the 32 bit to 64 bit transition some time ago. All the generic code
> assumes that 64-bit architectures always have 64-bit time_t
> (__ASSUME_TIME64_SYSCALLS).
>
> And the old interface is not exported and *only* meant to be used in
> compat symbols, different than the current way of handling y2038
> prof code (where the user might still define the ABI with _TIME_SIZE).
>
> That's why I think alpha compat code should be compartmentalized on
> alpha sysdep folder.
This doesn't address any of my concerns. It should not be necessary
to duplicate an *internal header* full of functions whose operation
is, or ought to be, completely generic, just because the exposed API
is different on Alpha.
zw
On 10/02/2020 22:10, Zack Weinberg wrote:
> On Mon, Feb 10, 2020 at 4:53 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 10/02/2020 18:35, Zack Weinberg wrote:
>>> On Mon, Feb 10, 2020 at 4:25 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>> On Mon, Feb 10, 2020 at 1:11 PM Zack Weinberg <zackw@panix.com> wrote:
>>>>>
>>>>> On Mon, Feb 10, 2020 at 12:50 PM Alistair Francis
>>>>> <alistair.francis@wdc.com> wrote:
>>>>>>
>>>>>> These functions are alpha specifc, rename them to be clear.
>>>>>
>>>>> These functions were meant to be generic. I put the file in
>>>>> s/u/s/l/alpha because it was only *used* on Alpha, at the time.
>>>>>
>>>>> What's stopping you from using these functions on all architectures?
>>>>
>>>> The Alpha functions use 32-bit versions of the timeval struct for a
>>>> 64-bit architecture. For all other architectures we want to use the
>>>> word size version (regardless of time_t size) for the timeval struct.
>>>> That's the main difference.
>>>
>>> I'm sorry, I haven't been following the discussion all that closely.
>>> Why is it necessary to use a 32-bit timeval struct (with 32-bit
>>> time_t, I presume) *only* on Alpha? Is there no way we can smooth
>>> over this difference so that, as you say,
>>
>> Alpha is an outlier here, it is the only 64-bit architecture that did
>> the 32 bit to 64 bit transition some time ago. All the generic code
>> assumes that 64-bit architectures always have 64-bit time_t
>> (__ASSUME_TIME64_SYSCALLS).
>>
>> And the old interface is not exported and *only* meant to be used in
>> compat symbols, different than the current way of handling y2038
>> prof code (where the user might still define the ABI with _TIME_SIZE).
>>
>> That's why I think alpha compat code should be compartmentalized on
>> alpha sysdep folder.
>
> This doesn't address any of my concerns. It should not be necessary
> to duplicate an *internal header* full of functions whose operation
> is, or ought to be, completely generic, just because the exposed API
> is different on Alpha.
The 32-bit timeval struct are alpha specific in a sense that no other
64-bit architectures have 32 time_t.
We could certainly make it generic and add even more internally
pre-processor magic to fit alpha code in generic definitions, but I
think it is really a worthless complication. It is a legacy API,
and it is highly unlikely that any other port will use such code.
I tend to see that generic code, such as the syscall consolidation
I have done over the years, should be defined when either you have
multiples ABIs that use it or if is the default for newer ports
(such as generic kABI). Outliers implementations, such osf alpha
syscalls, should be secluded in alpha-only code.
On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 10/02/2020 22:10, Zack Weinberg wrote:
> >
> > This doesn't address any of my concerns. It should not be necessary
> > to duplicate an *internal header* full of functions whose operation
> > is, or ought to be, completely generic, just because the exposed API
> > is different on Alpha.
>
> The 32-bit timeval struct are alpha specific in a sense that no other
> 64-bit architectures have 32 time_t.
>
> We could certainly make it generic and add even more internally
> pre-processor magic to fit alpha code in generic definitions, but I
> think it is really a worthless complication. It is a legacy API,
> and it is highly unlikely that any other port will use such code.
I think we're talking past each other. This patch is about
tv32-compat.h. tv32-compat.h contains conversion functions between
e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
time_t. I still don't see any reason why these conversion functions
need to be different on Alpha than they are on the 32-bit
architectures.
Alpha is unusual in that certain legacy code paths need to call these
conversion functions, but *what the functions do* should be identical
to what they do on 32-bit architectures. There should be no need for
preprocessor magic or anything.
zw
On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> > On 10/02/2020 22:10, Zack Weinberg wrote:
> > >
> > > This doesn't address any of my concerns. It should not be necessary
> > > to duplicate an *internal header* full of functions whose operation
> > > is, or ought to be, completely generic, just because the exposed API
> > > is different on Alpha.
> >
> > The 32-bit timeval struct are alpha specific in a sense that no other
> > 64-bit architectures have 32 time_t.
> >
> > We could certainly make it generic and add even more internally
> > pre-processor magic to fit alpha code in generic definitions, but I
> > think it is really a worthless complication. It is a legacy API,
> > and it is highly unlikely that any other port will use such code.
>
> I think we're talking past each other. This patch is about
> tv32-compat.h. tv32-compat.h contains conversion functions between
> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> time_t. I still don't see any reason why these conversion functions
The generic patches added by this series convert between a 64-bit
time_t and a wordsize time_t. Alpha is different in that it converts
between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
That is why Adhemerval is saying we would need some preprocessor magic
to ensure that for Alpha we always convert to/from a 32-bit time_t on
a 64-bit arch.
> need to be different on Alpha than they are on the 32-bit
> architectures.
My understanding is that Alpha is a 64-bit architecture (correct me if
I'm wrong, that's just from Wikipedia). The generic patches convert
time_t to the architectures wordsize before passing it to the kernel
(either 32-bit or 64-bit) while Alpha is different here.
>
> Alpha is unusual in that certain legacy code paths need to call these
> conversion functions, but *what the functions do* should be identical
> to what they do on 32-bit architectures. There should be no need for
> preprocessor magic or anything.
Yes, they are the same as the 32-bit architectures, but not the 64-bit
architectures. Everything else in this patch is generic for both
32-bit and 64-bit (except for Alpha).
Alistair
>
> zw
On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > >
> > > > This doesn't address any of my concerns. It should not be necessary
> > > > to duplicate an *internal header* full of functions whose operation
> > > > is, or ought to be, completely generic, just because the exposed API
> > > > is different on Alpha.
> > >
> > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > 64-bit architectures have 32 time_t.
> > >
> > > We could certainly make it generic and add even more internally
> > > pre-processor magic to fit alpha code in generic definitions, but I
> > > think it is really a worthless complication. It is a legacy API,
> > > and it is highly unlikely that any other port will use such code.
> >
> > I think we're talking past each other. This patch is about
> > tv32-compat.h. tv32-compat.h contains conversion functions between
> > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > time_t. I still don't see any reason why these conversion functions
>
> The generic patches added by this series convert between a 64-bit
> time_t and a wordsize time_t. Alpha is different in that it converts
> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
Yes, I understand that, but the definition of the 32-bit-time_t struct
timeval is
struct __timeval32 {
__time32_t tv_sec;
__suseconds_t tv_usec;
};
either way, isn't it? So the conversion functions shouldn't care?
(If that's not what the definition of the 32-bit-time_t struct timeval
is, then I would like to know why, in *excruciating* detail.)
zw
On 11/02/2020 13:23, Zack Weinberg wrote:
> On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
>>>
>>> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>> On 10/02/2020 22:10, Zack Weinberg wrote:
>>>>>
>>>>> This doesn't address any of my concerns. It should not be necessary
>>>>> to duplicate an *internal header* full of functions whose operation
>>>>> is, or ought to be, completely generic, just because the exposed API
>>>>> is different on Alpha.
>>>>
>>>> The 32-bit timeval struct are alpha specific in a sense that no other
>>>> 64-bit architectures have 32 time_t.
>>>>
>>>> We could certainly make it generic and add even more internally
>>>> pre-processor magic to fit alpha code in generic definitions, but I
>>>> think it is really a worthless complication. It is a legacy API,
>>>> and it is highly unlikely that any other port will use such code.
>>>
>>> I think we're talking past each other. This patch is about
>>> tv32-compat.h. tv32-compat.h contains conversion functions between
>>> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
>>> time_t. I still don't see any reason why these conversion functions
>>
>> The generic patches added by this series convert between a 64-bit
>> time_t and a wordsize time_t. Alpha is different in that it converts
>> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
>
> Yes, I understand that, but the definition of the 32-bit-time_t struct
> timeval is
>
> struct __timeval32 {
> __time32_t tv_sec;
> __suseconds_t tv_usec;
> };
>
> either way, isn't it? So the conversion functions shouldn't care?
>
> (If that's not what the definition of the 32-bit-time_t struct timeval
> is, then I would like to know why, in *excruciating* detail.)
Yes, but why expose this specific types on architectures that do not
really require it? Sure we could make this code generic, but again
this will be used solely by alpha.
On Tue, Feb 11, 2020 at 12:14 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 11/02/2020 13:23, Zack Weinberg wrote:
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> >>>
> >>> On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>> On 10/02/2020 22:10, Zack Weinberg wrote:
> >>>>>
> >>>>> This doesn't address any of my concerns. It should not be necessary
> >>>>> to duplicate an *internal header* full of functions whose operation
> >>>>> is, or ought to be, completely generic, just because the exposed API
> >>>>> is different on Alpha.
> >>>>
> >>>> The 32-bit timeval struct are alpha specific in a sense that no other
> >>>> 64-bit architectures have 32 time_t.
> >>>>
> >>>> We could certainly make it generic and add even more internally
> >>>> pre-processor magic to fit alpha code in generic definitions, but I
> >>>> think it is really a worthless complication. It is a legacy API,
> >>>> and it is highly unlikely that any other port will use such code.
> >>>
> >>> I think we're talking past each other. This patch is about
> >>> tv32-compat.h. tv32-compat.h contains conversion functions between
> >>> e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> >>> time_t. I still don't see any reason why these conversion functions
> >>
> >> The generic patches added by this series convert between a 64-bit
> >> time_t and a wordsize time_t. Alpha is different in that it converts
> >> between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> > struct __timeval32 {
> > __time32_t tv_sec;
> > __suseconds_t tv_usec;
> > };
> >
> > either way, isn't it? So the conversion functions shouldn't care?
> >
> > (If that's not what the definition of the 32-bit-time_t struct timeval
> > is, then I would like to know why, in *excruciating* detail.)
>
> Yes, but why expose this specific types on architectures that do not
> really require it? Sure we could make this code generic, but again
> this will be used solely by alpha.
Huh? Isn't it the exact same definition that would be used on the
32-bit architectures? All the variation should be hiding in the
definition of __time32_t?
zw
On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > <adhemerval.zanella@linaro.org> wrote:
> > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > >
> > > > > This doesn't address any of my concerns. It should not be necessary
> > > > > to duplicate an *internal header* full of functions whose operation
> > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > is different on Alpha.
> > > >
> > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > 64-bit architectures have 32 time_t.
> > > >
> > > > We could certainly make it generic and add even more internally
> > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > think it is really a worthless complication. It is a legacy API,
> > > > and it is highly unlikely that any other port will use such code.
> > >
> > > I think we're talking past each other. This patch is about
> > > tv32-compat.h. tv32-compat.h contains conversion functions between
> > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > time_t. I still don't see any reason why these conversion functions
> >
> > The generic patches added by this series convert between a 64-bit
> > time_t and a wordsize time_t. Alpha is different in that it converts
> > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
>
> Yes, I understand that, but the definition of the 32-bit-time_t struct
> timeval is
>
> struct __timeval32 {
> __time32_t tv_sec;
> __suseconds_t tv_usec;
> };
This is what the Alpha specific calls use.
The generic one uses (the name is confusing, I thought I had changed it)
struct __timeval32
{
long tv_sec; /* Seconds. */
long tv_usec; /* Microseconds. */
};
This is because, as I mentioned earlier, that the kernel expects the
time to be the wordsize, regardless of the time_t size (except for
Alpha).
Using long types means that we can do the same thing for 32-bit and
64-bit architectures.
My v1 patchset had a specific 32-bit only version (which didn't apply
to 64-bit arches) but this didn't work with ARM, see the discussion
here: https://patchwork.ozlabs.org/patch/1232973/
Alistair
>
> either way, isn't it? So the conversion functions shouldn't care?
>
> (If that's not what the definition of the 32-bit-time_t struct timeval
> is, then I would like to know why, in *excruciating* detail.)
>
> zw
On Tue, Feb 11, 2020 at 9:56 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > >
> > > > > > This doesn't address any of my concerns. It should not be necessary
> > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > is different on Alpha.
> > > > >
> > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > 64-bit architectures have 32 time_t.
> > > > >
> > > > > We could certainly make it generic and add even more internally
> > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > think it is really a worthless complication. It is a legacy API,
> > > > > and it is highly unlikely that any other port will use such code.
> > > >
> > > > I think we're talking past each other. This patch is about
> > > > tv32-compat.h. tv32-compat.h contains conversion functions between
> > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > time_t. I still don't see any reason why these conversion functions
> > >
> > > The generic patches added by this series convert between a 64-bit
> > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> > struct __timeval32 {
> > __time32_t tv_sec;
> > __suseconds_t tv_usec;
> > };
>
> This is what the Alpha specific calls use.
>
> The generic one uses (the name is confusing, I thought I had changed it)
>
> struct __timeval32
> {
> long tv_sec; /* Seconds. */
> long tv_usec; /* Microseconds. */
> };
I have changed the name of this struct to __timeval_long, hopefully
that fixes some of the confusion. The name was left over from v1.
Alistair
>
> This is because, as I mentioned earlier, that the kernel expects the
> time to be the wordsize, regardless of the time_t size (except for
> Alpha).
>
> Using long types means that we can do the same thing for 32-bit and
> 64-bit architectures.
>
> My v1 patchset had a specific 32-bit only version (which didn't apply
> to 64-bit arches) but this didn't work with ARM, see the discussion
> here: https://patchwork.ozlabs.org/patch/1232973/
>
> Alistair
>
> >
> > either way, isn't it? So the conversion functions shouldn't care?
> >
> > (If that's not what the definition of the 32-bit-time_t struct timeval
> > is, then I would like to know why, in *excruciating* detail.)
> >
> > zw
On Tue, Feb 11, 2020 at 1:04 PM Alistair Francis <alistair23@gmail.com> wrote:
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> > struct __timeval32 {
> > __time32_t tv_sec;
> > __suseconds_t tv_usec;
> > };
>
> This is what the Alpha specific calls use.
> The generic one uses (the name is confusing, I thought I had changed it)
>
> struct __timeval32
> {
> long tv_sec; /* Seconds. */
> long tv_usec; /* Microseconds. */
> };
>
> This is because, as I mentioned earlier, that the kernel expects the
> time to be the wordsize, regardless of the time_t size (except for
> Alpha).
I think that's profoundly wrong. I think we should be using a
definition along the lines of
struct __timeval32 {
__time32_t tv_sec;
__suseconds32_t tv_usec;
};
on all architectures, with all necessary variation handled within the
definitions of __time32_t and __suseconds32_t. That way it can all
live in bits/typesizes.h which has to be system-specific regardless.
Please consider this a sustained objection to the entire patchset, not
just this one patch, unless you can come back with a convincing reason
why the above is not possible or would actually make life more
complicated.
(Incidentally, since it is probably going to come up yet again if we
proceed down this path, I still believe that the specification of
`long tv_nsec` in struct timespec is a defect in C11 and POSIX and we
should ignore both standards and use a typedef name for it.)
zw
On Tue, Feb 11, 2020 at 10:38 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Tue, Feb 11, 2020 at 1:04 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > > timeval is
> > >
> > > struct __timeval32 {
> > > __time32_t tv_sec;
> > > __suseconds_t tv_usec;
> > > };
> >
> > This is what the Alpha specific calls use.
> > The generic one uses (the name is confusing, I thought I had changed it)
> >
> > struct __timeval32
> > {
> > long tv_sec; /* Seconds. */
> > long tv_usec; /* Microseconds. */
> > };
> >
> > This is because, as I mentioned earlier, that the kernel expects the
> > time to be the wordsize, regardless of the time_t size (except for
> > Alpha).
>
> I think that's profoundly wrong. I think we should be using a
> definition along the lines of
>
> struct __timeval32 {
> __time32_t tv_sec;
> __suseconds32_t tv_usec;
> };
>
> on all architectures, with all necessary variation handled within the
> definitions of __time32_t and __suseconds32_t. That way it can all
> live in bits/typesizes.h which has to be system-specific regardless.
When you say all architectures, you mean just 32-bit architectures and
Alpha right?
This is what my v1 did, and the general consensus was against that as
it didn't work for all 32-bit architectures (see the discussion here:
https://patchwork.ozlabs.org/patch/1232973/).
>
> Please consider this a sustained objection to the entire patchset, not
> just this one patch, unless you can come back with a convincing reason
> why the above is not possible or would actually make life more
> complicated.
What are you proposing I do instead?
>
> (Incidentally, since it is probably going to come up yet again if we
> proceed down this path, I still believe that the specification of
> `long tv_nsec` in struct timespec is a defect in C11 and POSIX and we
> should ignore both standards and use a typedef name for it.)
I'll leave this for others to comment on, I don't really mind either
way but ignoring the standard seems like a strange choice.
Alistair
>
> zw
11.02.2020 в 09:56:59 -0800 Alistair Francis написал(а):
> On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > >
> > > > > > This doesn't address any of my concerns. It should not be necessary
> > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > is different on Alpha.
> > > > >
> > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > 64-bit architectures have 32 time_t.
> > > > >
> > > > > We could certainly make it generic and add even more internally
> > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > think it is really a worthless complication. It is a legacy API,
> > > > > and it is highly unlikely that any other port will use such code.
> > > >
> > > > I think we're talking past each other. This patch is about
> > > > tv32-compat.h. tv32-compat.h contains conversion functions between
> > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > time_t. I still don't see any reason why these conversion functions
> > >
> > > The generic patches added by this series convert between a 64-bit
> > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> >
> > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > timeval is
> >
> > struct __timeval32 {
> > __time32_t tv_sec;
> > __suseconds_t tv_usec;
> > };
>
> This is what the Alpha specific calls use.
>
> The generic one uses (the name is confusing, I thought I had changed it)
>
> struct __timeval32
> {
> long tv_sec; /* Seconds. */
> long tv_usec; /* Microseconds. */
> };
This probably won't work for x32 and sparc64.
I suspect that it will be much simpler to have __timeval32 with 32-bit
fields and then do something like this:
int
__getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
{
#if KERNEL_OLD_TIMEVAL_IS_TIMEVAL64
return INLINE_SYSCALL_CALL (getitimer, which, &curr_value);
#else
struct __itimerval32 curr_value_32;
if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
return -1;
/* Write all fields of 'curr_value' regardless of overflow. */
curr_value->it_interval
= valid_timeval32_to_timeval64 (curr_value_32.it_interval);
curr_value->it_value
= valid_timeval32_to_timeval64 (curr_value_32.it_value);
return 0;
#endif
}
where KERNEL_OLD_TIMEVAL_IS_TIMEVAL64 needs to be true on 64-bit
architectures and x32.
On Tue, Feb 11, 2020 at 12:15 PM Stepan Golosunov
<stepan@golosunov.pp.ru> wrote:
>
> 11.02.2020 в 09:56:59 -0800 Alistair Francis написал(а):
> > On Tue, Feb 11, 2020 at 8:23 AM Zack Weinberg <zackw@panix.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 11, 2020 at 6:39 AM Zack Weinberg <zackw@panix.com> wrote:
> > > > >
> > > > > On Tue, Feb 11, 2020 at 8:25 AM Adhemerval Zanella
> > > > > <adhemerval.zanella@linaro.org> wrote:
> > > > > > On 10/02/2020 22:10, Zack Weinberg wrote:
> > > > > > >
> > > > > > > This doesn't address any of my concerns. It should not be necessary
> > > > > > > to duplicate an *internal header* full of functions whose operation
> > > > > > > is, or ought to be, completely generic, just because the exposed API
> > > > > > > is different on Alpha.
> > > > > >
> > > > > > The 32-bit timeval struct are alpha specific in a sense that no other
> > > > > > 64-bit architectures have 32 time_t.
> > > > > >
> > > > > > We could certainly make it generic and add even more internally
> > > > > > pre-processor magic to fit alpha code in generic definitions, but I
> > > > > > think it is really a worthless complication. It is a legacy API,
> > > > > > and it is highly unlikely that any other port will use such code.
> > > > >
> > > > > I think we're talking past each other. This patch is about
> > > > > tv32-compat.h. tv32-compat.h contains conversion functions between
> > > > > e.g. struct timeval with 32-bit time_t and struct timeval with 64-bit
> > > > > time_t. I still don't see any reason why these conversion functions
> > > >
> > > > The generic patches added by this series convert between a 64-bit
> > > > time_t and a wordsize time_t. Alpha is different in that it converts
> > > > between a 64-bit time_t and a 32-bit time_t on a 64-bit arch.
> > >
> > > Yes, I understand that, but the definition of the 32-bit-time_t struct
> > > timeval is
> > >
> > > struct __timeval32 {
> > > __time32_t tv_sec;
> > > __suseconds_t tv_usec;
> > > };
> >
> > This is what the Alpha specific calls use.
> >
> > The generic one uses (the name is confusing, I thought I had changed it)
> >
> > struct __timeval32
> > {
> > long tv_sec; /* Seconds. */
> > long tv_usec; /* Microseconds. */
> > };
>
> This probably won't work for x32 and sparc64.
>
> I suspect that it will be much simpler to have __timeval32 with 32-bit
> fields and then do something like this:
>
> int
> __getitimer64 (__itimer_which_t which, struct __itimerval64 *curr_value)
> {
> #if KERNEL_OLD_TIMEVAL_IS_TIMEVAL64
> return INLINE_SYSCALL_CALL (getitimer, which, &curr_value);
> #else
> struct __itimerval32 curr_value_32;
> if (INLINE_SYSCALL_CALL (getitimer, which, &curr_value_32) == -1)
> return -1;
>
> /* Write all fields of 'curr_value' regardless of overflow. */
> curr_value->it_interval
> = valid_timeval32_to_timeval64 (curr_value_32.it_interval);
> curr_value->it_value
> = valid_timeval32_to_timeval64 (curr_value_32.it_value);
> return 0;
> #endif
> }
>
> where KERNEL_OLD_TIMEVAL_IS_TIMEVAL64 needs to be true on 64-bit
> architectures and x32.
This works for me.
The files apply to all architectures (so we don't have the issues
where it doesn't take effect for ARM). It fixes the RV32 issues and it
means I can cleanup the Alpha code.
I have made these changes and am testing now.
Alistair
similarity index 88%
rename from sysdeps/unix/sysv/linux/alpha/tv32-compat.h
rename to sysdeps/unix/sysv/linux/alpha/alpha-tv32-compat.h
@@ -70,13 +70,13 @@ struct rusage32
overflow, they write { INT32_MAX, TV_USEC_MAX } to the output. */
static inline struct timeval
-valid_timeval32_to_timeval (const struct timeval32 tv)
+alpha_valid_timeval32_to_timeval (const struct timeval32 tv)
{
return (struct timeval) { tv.tv_sec, tv.tv_usec };
}
static inline struct timeval32
-valid_timeval_to_timeval32 (const struct timeval tv64)
+alpha_valid_timeval_to_timeval32 (const struct timeval tv64)
{
if (__glibc_unlikely (tv64.tv_sec > (time_t) INT32_MAX))
return (struct timeval32) { INT32_MAX, TV_USEC_MAX};
@@ -84,27 +84,27 @@ valid_timeval_to_timeval32 (const struct timeval tv64)
}
static inline struct timespec
-valid_timeval32_to_timespec (const struct timeval32 tv)
+alpha_valid_timeval32_to_timespec (const struct timeval32 tv)
{
return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 };
}
static inline struct timeval32
-valid_timespec_to_timeval32 (const struct timespec ts)
+alpha_valid_timespec_to_timeval32 (const struct timespec ts)
{
return (struct timeval32) { (time_t) ts.tv_sec, ts.tv_nsec / 1000 };
}
static inline void
-rusage64_to_rusage32 (struct rusage32 *restrict r32,
+alpha_rusage64_to_rusage32 (struct rusage32 *restrict r32,
const struct rusage *restrict r64)
{
/* Make sure the entire output structure is cleared, including
padding and reserved fields. */
memset (r32, 0, sizeof *r32);
- r32->ru_utime = valid_timeval_to_timeval32 (r64->ru_utime);
- r32->ru_stime = valid_timeval_to_timeval32 (r64->ru_stime);
+ r32->ru_utime = alpha_valid_timeval_to_timeval32 (r64->ru_utime);
+ r32->ru_stime = alpha_valid_timeval_to_timeval32 (r64->ru_stime);
r32->ru_maxrss = r64->ru_maxrss;
r32->ru_ixrss = r64->ru_ixrss;
r32->ru_idrss = r64->ru_idrss;
@@ -121,4 +121,4 @@ rusage64_to_rusage32 (struct rusage32 *restrict r32,
r32->ru_nivcsw = r64->ru_nivcsw;
}
-#endif /* tv32-compat.h */
+#endif /* alpha-tv32-compat.h */
@@ -22,7 +22,7 @@
#include <sys/time.h>
#include <sys/timex.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
struct timex32 {
unsigned int modes; /* mode selector */
@@ -57,13 +57,13 @@ int
attribute_compat_text_section
__adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
{
- struct timeval itv64 = valid_timeval32_to_timeval (*itv);
+ struct timeval itv64 = alpha_valid_timeval32_to_timeval (*itv);
struct timeval otv64;
if (__adjtime (&itv64, &otv64) == -1)
return -1;
- *otv = valid_timeval_to_timeval32 (otv64);
+ *otv = alpha_valid_timeval_to_timeval32 (otv64);
return 0;
}
@@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
tx64.calcnt = tx->calcnt;
tx64.errcnt = tx->errcnt;
tx64.stbcnt = tx->stbcnt;
- tx64.time = valid_timeval32_to_timeval (tx->time);
+ tx64.time = alpha_valid_timeval32_to_timeval (tx->time);
int status = __adjtimex (&tx64);
if (status < 0)
@@ -116,7 +116,7 @@ __adjtimex_tv32 (struct timex32 *tx)
tx->calcnt = tx64.calcnt;
tx->errcnt = tx64.errcnt;
tx->stbcnt = tx64.stbcnt;
- tx->time = valid_timeval_to_timeval32 (tx64.time);
+ tx->time = alpha_valid_timeval_to_timeval32 (tx64.time);
return status;
}
@@ -21,7 +21,7 @@
#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
#include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
int
attribute_compat_text_section
@@ -33,9 +33,9 @@ __getitimer_tv32 (int which, struct itimerval32 *curr_value)
/* Write all fields of 'curr_value' regardless of overflow. */
curr_value->it_interval
- = valid_timeval_to_timeval32 (curr_value_64.it_interval);
+ = alpha_valid_timeval_to_timeval32 (curr_value_64.it_interval);
curr_value->it_value
- = valid_timeval_to_timeval32 (curr_value_64.it_value);
+ = alpha_valid_timeval_to_timeval32 (curr_value_64.it_value);
return 0;
}
@@ -22,7 +22,7 @@
#include <sys/time.h>
#include <sys/resource.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
int
__getrusage_tv32 (int who, struct rusage32 *usage32)
@@ -31,7 +31,7 @@ __getrusage_tv32 (int who, struct rusage32 *usage32)
if (__getrusage (who, &usage64) == -1)
return -1;
- rusage64_to_rusage32 (usage32, &usage64);
+ alpha_rusage64_to_rusage32 (usage32, &usage64);
return 0;
}
@@ -23,7 +23,7 @@
#include <string.h>
#include <time.h>
#include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
/* Get the current time of day and timezone information putting it
into *TV and *TZ. */
@@ -38,7 +38,7 @@ __gettimeofday_tv32 (struct timeval32 *restrict tv32, void *restrict tz)
struct timespec ts;
__clock_gettime (CLOCK_REALTIME, &ts);
- *tv32 = valid_timespec_to_timeval32 (ts);
+ *tv32 = alpha_valid_timespec_to_timeval32 (ts);
return 0;
}
@@ -21,7 +21,7 @@
#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
#include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
int
attribute_compat_text_section
@@ -30,9 +30,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
{
struct itimerval new_value_64;
new_value_64.it_interval
- = valid_timeval32_to_timeval (new_value->it_interval);
+ = alpha_valid_timeval32_to_timeval (new_value->it_interval);
new_value_64.it_value
- = valid_timeval32_to_timeval (new_value->it_value);
+ = alpha_valid_timeval32_to_timeval (new_value->it_value);
if (old_value == NULL)
return __setitimer (which, &new_value_64, NULL);
@@ -43,9 +43,9 @@ __setitimer_tv32 (int which, const struct itimerval32 *restrict new_value,
/* Write all fields of 'old_value' regardless of overflow. */
old_value->it_interval
- = valid_timeval_to_timeval32 (old_value_64.it_interval);
+ = alpha_valid_timeval_to_timeval32 (old_value_64.it_interval);
old_value->it_value
- = valid_timeval_to_timeval32 (old_value_64.it_value);
+ = alpha_valid_timeval_to_timeval32 (old_value_64.it_value);
return 0;
}
@@ -23,7 +23,7 @@
#include <sys/time.h>
#include <time.h>
#include <errno.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
/* Set the current time of day and timezone information.
This call is restricted to the super-user. */
@@ -42,7 +42,7 @@ __settimeofday_tv32 (const struct timeval32 *tv32,
return __settimezone (tz);
}
- struct timespec ts = valid_timeval32_to_timespec (*tv32);
+ struct timespec ts = alpha_valid_timeval32_to_timespec (*tv32);
return __clock_settime (CLOCK_REALTIME, &ts);
}
@@ -21,15 +21,15 @@
#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
#include <sys/time.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
int
attribute_compat_text_section
__utimes_tv32 (const char *filename, const struct timeval32 times32[2])
{
struct timeval times[2];
- times[0] = valid_timeval32_to_timeval (times32[0]);
- times[1] = valid_timeval32_to_timeval (times32[1]);
+ times[0] = alpha_valid_timeval32_to_timeval (times32[0]);
+ times[1] = alpha_valid_timeval32_to_timeval (times32[1]);
return __utimes (filename, times);
}
@@ -23,7 +23,7 @@
#include <sys/time.h>
#include <sys/resource.h>
#include <sys/wait.h>
-#include <tv32-compat.h>
+#include <alpha-tv32-compat.h>
pid_t
attribute_compat_text_section
@@ -33,7 +33,7 @@ __wait4_tv32 (pid_t pid, int *status, int options, struct rusage32 *usage32)
pid_t child = __wait4 (pid, status, options, &usage64);
if (child >= 0 && usage32 != NULL)
- rusage64_to_rusage32 (usage32, &usage64);
+ alpha_rusage64_to_rusage32 (usage32, &usage64);
return child;
}