Re: [PATCH] sysv: linux: Pass 64-bit version of semctl syscall

[Arnd Bergmann <arnd at arndb dot de>]

On Sat, Feb 1, 2020 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Fri, Jan 31, 2020 at 11:58 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sat, Feb 1, 2020 at 12:59 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> > > > <alistair.francis@wdc.com> wrote:
> > > > >
> > > > > Adjust the semctl syscall to match what the kernel expects. That is pass
> > > > > a version with a *_high version of sem_otime and sem_ctime in the order
> > > > > that the kernel expects.
> > > >
> > > > > +# ifdef __SEMID_DS_HIGH
> > > > > +#  if defined (__SEMID_DS_HIGH_END)
> > > > ...
> > > > > +#  elif defined (__SEMID_DS_HIGH_SWAP)
> > > >
> > > > The three new macros you check for here directly correspond to the
> > > > possible combinations of the existing __SEM_PAD_AFTER_TIME
> > > > and __SEM_PAD_BEFORE_TIME macros, right?
> > >
> > > Hey Arnd,
> > >
> > > I think you are asking why we don't just use the existing macros
> > > instead of adding more (and keep the core logic as is). Besides the
> > > naming being confusing then, they two types don't line up. For
> > > example:
> > >
> > > PowerPC has this:
> > >
> > >  #define __SEM_PAD_AFTER_TIME 0
> > >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > >
> > > while HPPA has this:
> > >
> > >  #define __SEM_PAD_AFTER_TIME 0
> > >  #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > >
> > > They have different __SEMID_DS macros, but the same __SEM_PAD macros.
> >
> > If they don't line up, that probably means that one of them is wrong ;-)
>
> Ha, ok I'll double check and update it.
>
> Even still, I would rather not use the existing macros as they use
> __TIMESIZE == 32 and the new ones are using __WORDSIZE == 32.

Ah, I had not noticed that difference.

> We could of course just have a RV32 override that fixes it for RV32, but
> then we need another override.
>
> The main reason not to is that I think it would be confusing to
> overload the macro like that and I don't see a disadvantage in having
> a new macro. It makes what is going on (which is already somewhat
> confusing) more clear.

Makes sense.

Another idea would be to not define a separate struct at all, except for
mips (which needs special handling anyway), and then define a
new macro fro the few targets that do need to swap the one word
with __TIMESIZE==64: s390-32, arm/be, sh/be, m68k and microblaze/be
and do nothing on the others.

       Arnd