Re: [PATCH 5/7] linux: Use waitid on wait4 if __NR_wait4 is not defined

[Alistair Francis <alistair23@gmail.com>]

On Fri, Nov 22, 2019 at 4:15 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 21/11/2019 15:41, Alistair Francis wrote:
> > On Thu, Nov 21, 2019 at 9:53 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 14/11/2019 11:47, Adhemerval Zanella wrote:
> >>> +pid_t
> >>> +__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
> >>> +{
> >>> +#if __NR_wait4
> >>> +   return SYSCALL_CANCEL (wait4, pid, stat_loc, options, usage);
> >>> +#elif defined (__ASSUME_WAITID_PID0_P_PGID)
> >> [...]
> >>> +# else
> >>> +/* Linux waitid prior kernel 5.4 does not support waiting for the current
> >>> +   process.  It would be possible to emulate it by calling getpgid for pid 0,
> >>> +   however, it would require an additional syscall and it is inherent racy:
> >>> +   after the current process group is received and before it is passed
> >>> +   to waitid a signal could arrive causing the current process group to
> >>> +   change.  */
> >>> +# error "The kernel ABI does not provide a way to implement wait4"
> >>> +#endif
> >>
> >> So the only design here that I am not sure is if the best one is to trigger
> >> a build error to avoid an architecture to not define __NR_wait4 and also
> >> support kernels older than 5.4 (which would not define
> >> __ASSUME_WAITID_PID0_P_PGID), or if it should do as generic implementation
> >> and return ENOSYS along with a stub.
> >>
> >> Thoughts?
> >
> > I think a build error makes sense. Currently only RV32 doesn't have
> > __NR_wait4 (which isn't upstreamed) so you aren't breaking anything.
> >
> > The only kernels that could possibly not have __NR_wait4 and be less
> > then 5.4 are 5.1, 5.2 and 5.3, non of which are stable so they will
> > slowly disappear anyway.
> >
> > Not producing a build error could be very confusing for developers
> > that do get bitten by the missing implementation.
> >
>
> My point if if checking for kernel version to define __ASSUME_WAITID_PID0_P_PGID
> does make, meaning it is possible with some config option in the kernel
> to enable only waitid for kernels older than 5.3; or if we can assume
> some configuration in always invalid and thus the kernel won't allow
> enable it.
>
> If the latter we can then remove the __ASSUME_WAITID_PID0_P_PGID and
> add a comment on waitid implementation stating that if waitid is the
> only syscall supported then it is suppose to be the superset of all
> wait* functionalities.

I think I understand what you are saying.

It is NOT the case that if waitid is the only syscall supported then
it is a superset of all wait* functions.

For RV32 the 5.1, 5.2 and 5.3 only support waitid but do not support
the PID0 P_PGID functionality. In these three kernel cases the call
will fail.

Alistair