| Message ID | 20230827155746.84781-12-kariem.taha2.7@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | bsd-user: Implement freebsd process related system calls. | expand |
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. 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. r~
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
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; +} + diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h index 048773a75d..b6225e520e 100644 --- a/bsd-user/bsd-proc.h +++ b/bsd-user/bsd-proc.h @@ -26,6 +26,8 @@ #include "gdbstub/syscalls.h" #include "qemu/plugin.h" +int bsd_get_ncpu(void); + /* exit(2) */ static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1) {