Re: [PATCH 3/6] y2038: Introduce struct __timeval64 - new internal glibc type

[Lukasz Majewski <lukma at denx dot de>]

Hi Arnd, Adhemerval,

> On Mon, Jan 20, 2020 at 5:46 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> > On 20/01/2020 10:00, Arnd Bergmann wrote:  
> > > On Mon, Jan 20, 2020 at 1:34 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > >>>> On Sun, Jan 19, 2020 at 4:22 PM Lukasz Majewski <lukma@denx.de>
> > >>>> wrote:  
> > >>>>>> On Sat, Jan 18, 2020 at 11:48 PM Alistair Francis
> > >>>>>> <alistair23@gmail.com> wrote:  
> > >>>>
> > >>>> It would certainly help if this could be consistent across
> > >>>> architectures and libraries.  
> > >>>
> > >>> Ok. Thanks for the input.  
> > >>
> > >> Would it be OK to have:
> > >>
> > >> struct __timeval64
> > >> {
> > >>   __time64_t tv_sec;         /* Seconds */
> > >>   __int64_t tv_usec;       /* Microseconds */
> > >> };  
> > >
> > > This would not work if you pass it into a kernel interface that
> > > expects a 32-bit suseconds_t on sparc64, but it should work as an
> > > internal type on all 32-bit architectures.
> > >  
> > >> I would prefer to avoid changing typedef of suseconds_t as it may
> > >> affect struct timeval related operations.
> > >>
> > >> Using explicitly __int64_t for tv_usec seems better option,
> > >> isn't it?  
> > >
> > > Maybe a __suseconds64_t that happens to be 32-bit wide on
> > > sparc64?  
> >
> > If timeval64 kernel ABI expects tv_usec being a different type
> > depending of the architecture, it is an indication we should
> > parametrize with a __suseconds64_t.
> >
> > As a side note, why tv_usec for timeval64 is a 64-bit value? Is it
> > related to some alignment constraint?  
> 
> I think it's mostly for simplicity: we do need the microseconds to be
> in the low bytes for compatibility with the 64-bit syscalls in the
> kernel, so it's either a 64-bit microseconds value or the same
> conditional padding that exists in timespec, with an extra special
> case for sparc64.
> 
> The 64-bit case is slightly simpler for the few kernel interfaces that
> take a timeval as input and then can avoid conditionally zeroing out
> the upper 32 bits before checking the lower 32 bits again 1000000.
> I think that is only needed for clock_adjtimex, VIDIOC_QBUF and
> PPSETTIME though, everything else only passes a timeval
> to user space.

I would opt for simpler approach :-)

Is the below code correct?
./include/time.h

struct __timeval64
{
   __time64_t tv_sec;         /* Seconds */
   __suseconds64_t tv_usec;       /* Microseconds */
};

However, I'm a bit confused on the correct place to define
__suseconds64_t.

Shall it be added to posix/bits/types.h?
__STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of
microseconds.  */

If yes, then wouldn't we break the compatibility by adding new type to
this file?

> 
>        Arnd




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

Attachment: pgppxn7oCM35b.pgp
Description: OpenPGP digital signature