[PATCH nginx v2] remove ngx_cpuinfo, use sysconf or define
Vladimir Homutov
vl at nginx.com
Thu Jan 16 15:46:45 UTC 2020
On Thu, Jan 16, 2020 at 07:39:07AM -0800, Joe Konno wrote:
> Scroll to bottom:
>
> On 1/16/20 6:43 AM, Vladimir Homutov wrote:
> > On Fri, Jan 03, 2020 at 02:39:29PM -0800, Joe Konno wrote:
> >> # HG changeset patch
> >> # User Joe Konno <joe.konno at intel.com>
> >> # Date 1578075036 28800
> >> # Fri Jan 03 10:10:36 2020 -0800
> >> # Node ID b66ee34cea2f539bb8ce4986d6bd332f75ee06b6
> >> # Parent f1720934c45be1c6469841a55d1f31fe9a630c85
> >> remove ngx_cpuinfo, use sysconf or define
> >>
> >> Function ngx_cpuinfo() is used to (sub-optimally) determine the value of
> >> ngx_cacheline_size on x86 hardware. On hardware running Linux, there may be a
> >> sysconf call that correctly reports the cache line size using architecturally
> >> appropriate methods-- for x86 hardware, the CPUID instruction.
> >>
> >> Therefore, ngx_cpuinfo is no longer needed. If said sysconf call is
> >> unavailable, set ngx_cachline_size to defined NGX_CPU_CACHE_LINE, the value of
> >> which is determined at configure time.
> >>
> >> Signed-off-by: Joe Konno <joe.konno at intel.com>
> >>
> >> diff -r f1720934c45b -r b66ee34cea2f auto/sources
> >> --- a/auto/sources Fri Dec 27 19:43:01 2019 +0300
> >> +++ b/auto/sources Fri Jan 03 10:10:36 2020 -0800
> >> @@ -71,7 +71,6 @@
> >> src/core/ngx_cycle.c \
> >> src/core/ngx_spinlock.c \
> >> src/core/ngx_rwlock.c \
> >> - src/core/ngx_cpuinfo.c \
> >> src/core/ngx_conf_file.c \
> >> src/core/ngx_module.c \
> >> src/core/ngx_resolver.c \
> >> diff -r f1720934c45b -r b66ee34cea2f src/core/ngx_core.h
> >> --- a/src/core/ngx_core.h Fri Dec 27 19:43:01 2019 +0300
> >> +++ b/src/core/ngx_core.h Fri Jan 03 10:10:36 2020 -0800
> >> @@ -102,7 +102,6 @@
> >> #define ngx_max(val1, val2) ((val1 < val2) ? (val2) : (val1))
> >> #define ngx_min(val1, val2) ((val1 > val2) ? (val2) : (val1))
> >>
> >> -void ngx_cpuinfo(void);
> >>
> >> #if (NGX_HAVE_OPENAT)
> >> #define NGX_DISABLE_SYMLINKS_OFF 0
> >> diff -r f1720934c45b -r b66ee34cea2f src/core/ngx_cpuinfo.c
> >> --- a/src/core/ngx_cpuinfo.c Fri Dec 27 19:43:01 2019 +0300
> >> +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
> >> @@ -1,139 +0,0 @@
> >> -
> >> -/*
> >> - * Copyright (C) Igor Sysoev
> >> - * Copyright (C) Nginx, Inc.
> >> - */
> >> -
> >> -
> >> -#include <ngx_config.h>
> >> -#include <ngx_core.h>
> >> -
> >> -
> >> -#if (( __i386__ || __amd64__ ) && ( __GNUC__ || __INTEL_COMPILER ))
> >> -
> >> -
> >> -static ngx_inline void ngx_cpuid(uint32_t i, uint32_t *buf);
> >> -
> >> -
> >> -#if ( __i386__ )
> >> -
> >> -static ngx_inline void
> >> -ngx_cpuid(uint32_t i, uint32_t *buf)
> >> -{
> >> -
> >> - /*
> >> - * we could not use %ebx as output parameter if gcc builds PIC,
> >> - * and we could not save %ebx on stack, because %esp is used,
> >> - * when the -fomit-frame-pointer optimization is specified.
> >> - */
> >> -
> >> - __asm__ (
> >> -
> >> - " mov %%ebx, %%esi; "
> >> -
> >> - " cpuid; "
> >> - " mov %%eax, (%1); "
> >> - " mov %%ebx, 4(%1); "
> >> - " mov %%edx, 8(%1); "
> >> - " mov %%ecx, 12(%1); "
> >> -
> >> - " mov %%esi, %%ebx; "
> >> -
> >> - : : "a" (i), "D" (buf) : "ecx", "edx", "esi", "memory" );
> >> -}
> >> -
> >> -
> >> -#else /* __amd64__ */
> >> -
> >> -
> >> -static ngx_inline void
> >> -ngx_cpuid(uint32_t i, uint32_t *buf)
> >> -{
> >> - uint32_t eax, ebx, ecx, edx;
> >> -
> >> - __asm__ (
> >> -
> >> - "cpuid"
> >> -
> >> - : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx) : "a" (i) );
> >> -
> >> - buf[0] = eax;
> >> - buf[1] = ebx;
> >> - buf[2] = edx;
> >> - buf[3] = ecx;
> >> -}
> >> -
> >> -
> >> -#endif
> >> -
> >> -
> >> -/* auto detect the L2 cache line size of modern and widespread CPUs */
> >> -
> >> -void
> >> -ngx_cpuinfo(void)
> >> -{
> >> - u_char *vendor;
> >> - uint32_t vbuf[5], cpu[4], model;
> >> -
> >> - vbuf[0] = 0;
> >> - vbuf[1] = 0;
> >> - vbuf[2] = 0;
> >> - vbuf[3] = 0;
> >> - vbuf[4] = 0;
> >> -
> >> - ngx_cpuid(0, vbuf);
> >> -
> >> - vendor = (u_char *) &vbuf[1];
> >> -
> >> - if (vbuf[0] == 0) {
> >> - return;
> >> - }
> >> -
> >> - ngx_cpuid(1, cpu);
> >> -
> >> - if (ngx_strcmp(vendor, "GenuineIntel") == 0) {
> >> -
> >> - switch ((cpu[0] & 0xf00) >> 8) {
> >> -
> >> - /* Pentium */
> >> - case 5:
> >> - ngx_cacheline_size = 32;
> >> - break;
> >> -
> >> - /* Pentium Pro, II, III */
> >> - case 6:
> >> - ngx_cacheline_size = 32;
> >> -
> >> - model = ((cpu[0] & 0xf0000) >> 8) | (cpu[0] & 0xf0);
> >> -
> >> - if (model >= 0xd0) {
> >> - /* Intel Core, Core 2, Atom */
> >> - ngx_cacheline_size = 64;
> >> - }
> >> -
> >> - break;
> >> -
> >> - /*
> >> - * Pentium 4, although its cache line size is 64 bytes,
> >> - * it prefetches up to two cache lines during memory read
> >> - */
> >> - case 15:
> >> - ngx_cacheline_size = 128;
> >> - break;
> >> - }
> >> -
> >> - } else if (ngx_strcmp(vendor, "AuthenticAMD") == 0) {
> >> - ngx_cacheline_size = 64;
> >> - }
> >> -}
> >> -
> >> -#else
> >> -
> >> -
> >> -void
> >> -ngx_cpuinfo(void)
> >> -{
> >> -}
> >> -
> >> -
> >> -#endif
> >> diff -r f1720934c45b -r b66ee34cea2f src/os/unix/ngx_posix_init.c
> >> --- a/src/os/unix/ngx_posix_init.c Fri Dec 27 19:43:01 2019 +0300
> >> +++ b/src/os/unix/ngx_posix_init.c Fri Jan 03 10:10:36 2020 -0800
> >> @@ -51,10 +51,20 @@
> >> }
> >>
> >> ngx_pagesize = getpagesize();
> >> - ngx_cacheline_size = NGX_CPU_CACHE_LINE;
> >>
> >> for (n = ngx_pagesize; n >>= 1; ngx_pagesize_shift++) { /* void */ }
> >>
> >> +#if (NGX_HAVE_LEVEL1_DCACHE_LINESIZE)
> >> + size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> >> + if (size > 0) {
> >> + ngx_cacheline_size = size;
> >> + } else {
> >> + ngx_cacheline_size = NGX_CPU_CACHE_LINE;
> >> + }
> >> +#else
> >> + ngx_cacheline_size = NGX_CPU_CACHE_LINE;
> >> +#endif
> >> +
> >> #if (NGX_HAVE_SC_NPROCESSORS_ONLN)
> >> if (ngx_ncpu == 0) {
> >> ngx_ncpu = sysconf(_SC_NPROCESSORS_ONLN);
> >> @@ -65,15 +75,6 @@
> >> ngx_ncpu = 1;
> >> }
> >>
> >> -#if (NGX_HAVE_LEVEL1_DCACHE_LINESIZE)
> >> - size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> >> - if (size > 0) {
> >> - ngx_cacheline_size = size;
> >> - }
> >> -#endif
> >> -
> >> - ngx_cpuinfo();
> >> -
> >> if (getrlimit(RLIMIT_NOFILE, &rlmt) == -1) {
> >> ngx_log_error(NGX_LOG_ALERT, log, errno,
> >> "getrlimit(RLIMIT_NOFILE) failed");
> >>
> >
> > Thank you for sharing!
> >
> > Removing ngx_cpuinfo() doesn't look correct: by doing this you deprive
> > non-Linux systems (BSDs, windows and some others) support for CPU detection.
>
> Valid point. Thanks for reviewing.
>
> v1 of that patch used CPUID to retrieve the cache line size directly
> from H/W:
>
> http://mailman.nginx.org/pipermail/nginx-devel/2019-December/012853.html
yes, I've seen this too.
Although our current ngx_cpuinfo() may seem a bit outdated, it does its
job, and contains some manually crafted cases. What we don't want is to
make things more complex here and introduce more very specific code.
If you know about specific cases when things get wrong with current
implementation, please report.
More information about the nginx-devel
mailing list