Re: [PATCH v2 1/6] sysv/linux: Rename alpha functions to be alpha specific

[Alistair Francis <alistair23 at gmail dot com>]

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