Re: [PATCH 11/32] bsd-user: Get number of cpus.

[Warner Losh <imp@bsdimp.com>]

[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]

On Tue, Aug 29, 2023 at 1:50 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/27/23 08:57, Karim Taha wrote:
> > From: Kyle Evans <kevans@FreeBSD.org>
> >
> > Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> > Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> > ---
> >   bsd-user/bsd-proc.c | 39 +++++++++++++++++++++++++++++++++++++++
> >   bsd-user/bsd-proc.h |  2 ++
> >   2 files changed, 41 insertions(+)
> >
> > diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
> > index 49c0fb67d0..dd6bad6de3 100644
> > --- a/bsd-user/bsd-proc.c
> > +++ b/bsd-user/bsd-proc.c
> > @@ -185,3 +185,42 @@ int host_to_target_waitstatus(int status)
> >       return status;
> >   }
> >
> > +int bsd_get_ncpu(void)
> > +{
> > +    static int ncpu = -1;
> > +
> > +    if (ncpu != -1) {
> > +        return ncpu;
> > +    }
> > +    if (ncpu == -1) {
> > +        cpuset_t mask;
> > +
> > +        CPU_ZERO(&mask);
> > +
> > +        if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, -1,
> sizeof(mask),
> > +                               &mask) == 0) {
> > +            ncpu = CPU_COUNT(&mask);
> > +        }
> > +    }
> > +#ifdef _SC_NPROCESSORS_ONLN
> > +    if (ncpu == -1)
> > +        ncpu = sysconf(_SC_NPROCESSORS_ONLN);
> > +#endif
> > +#if defined(CTL_HW) && defined(HW_NCPU)
> > +    if (ncpu == -1) {
> > +        int mib[2] = {CTL_HW, HW_NCPU};
> > +        size_t sz;
> > +
> > +        sz = sizeof(ncpu);
> > +        if (sysctl(mib, 2, &ncpu, &sz, NULL, NULL) == -1) {
> > +            ncpu = -1;
> > +        }
> > +    }
> > +#endif
> > +    if (ncpu == -1) {
> > +        gemu_log("XXX Missing bsd_get_ncpu() implementation\n");
> > +        ncpu = 1;
> > +    }
> > +    return ncpu;
> > +}
>
> This has the look of odd compatibility code.  Surely all three of these
> alternatives are
> functional, and that sysconf() is easiest to use.
>

This code dates to the earliest days of the emulator when it ran on all
three BSDs. NetBSD
does support _SC_NPROCESSORS_ONLN, so we should leave that case.

I think the getaffinity stuff is there so that one can restrict a process
group to a subset of
the CPUs in the system for nicer build farms, but I could be mistaken about
that.  NetBSD
doesn't support this call, AFAICT, but I'd rather not add #ifdef's for
NetBSD until we actually
do a NetBSD port. I'll have to check with Kyle to see if that was really
needed, or if the
code was cut and pasted from elsewhere.

I don't think we need to fall back to the 4.4BSD hw.ncpu sysctl. Everybody
supports the sysconf
interface.


> Looking at the freebsd implementation of sysconf, it uses AT_NCPUS if
> available, so the
> value is already cached within the process in the common case.  So I also
> don't see a need
> for the ncpu local static either.
>

I agree with this... We only use it to impelment hw.ncpu emulation, and to
set AT_NCPUS
when we load, so who cares if it's expensive :).

Warner

[-- Attachment #2: Type: text/html, Size: 4212 bytes --]