From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 160ED3858D20 for ; Mon, 7 Feb 2022 11:25:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 160ED3858D20 Received: by mail-ot1-x329.google.com with SMTP id x52-20020a05683040b400b0059ea92202daso10618291ott.7 for ; Mon, 07 Feb 2022 03:25:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=ITfC4eTL2sfRWz05YjhEml8uTMtpGfsyf+k7lZj9hCw=; b=4DVuUYA+eTFcej8tm1LoGcREO7DzUr67xYDRpMoHIlAXxDW5LgylUtUviO0aLznXAN N6p9itoN+pYtbHGytfbl0GNP4KtzujEOppUFcnTdtd3oKkZGmUZShqdULgX4fqdB1ciY m+jnHOwddUXP8KWDn45MzRCQKOmfnnL88CvwRPTiJwQNnhGEYf4XRut/CHqkZYLlANRz FnbYJjiGAwzuPAEPEe+3t3S5RcHCoUpIYYan+tHrul5oaqwHMGlB9FK0QdRKwMEwH+Cp /Vuqn+4zM9c23NZSJZKUqr4e6BUn7xePt00aMpq68gwwjt7LqHnry9msq6TS/jDcPFbp 8mww== X-Gm-Message-State: AOAM5338nkg8Ls6Jcy9896UEbqeVI5tykPYdqR999/MTzVHQP/Gzr75E HSpbTOoDkA0GutqIZJ+WzYJuRRKuDEkitA== X-Google-Smtp-Source: ABdhPJw1WpmJkGIc4eki16mbN3r87/u5E/xq1RXcby++o2gzZWILstUxN9scMKw9MjUCJ2MtEWa4Bw== X-Received: by 2002:a05:6830:2685:: with SMTP id l5mr3899458otu.203.1644233113369; Mon, 07 Feb 2022 03:25:13 -0800 (PST) Received: from ?IPV6:2804:431:c7ca:733:4cdc:e08a:54c6:5108? ([2804:431:c7ca:733:4cdc:e08a:54c6:5108]) by smtp.gmail.com with ESMTPSA id t16sm3783075otc.29.2022.02.07.03.25.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Feb 2022 03:25:13 -0800 (PST) Message-ID: <2f8633c5-6335-b7aa-e735-65dc36322d7f@linaro.org> Date: Mon, 7 Feb 2022 08:25:11 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 Subject: Re: [PATCH] linux: fix accuracy of get_nprocs and get_nprocs_conf [BZ #28865] Content-Language: en-US To: "Dmitry V. Levin" , libc-alpha@sourceware.org References: <20220205212402.GA5233@altlinux.org> From: Adhemerval Zanella In-Reply-To: <20220205212402.GA5233@altlinux.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Feb 2022 11:25:16 -0000 On 05/02/2022 18:24, Dmitry V. Levin wrote: > get_nprocs() and get_nprocs_conf() use various methods to obtain an > accurate number of processors. Re-introduce __get_nprocs_sched() as > a source of information, and fix the order in which these methods are > used to return the most accurate information. The primary source of > information used in both functions remains unchanged. > > This also changes __get_nprocs_sched() error return value from 2 to 0, > but all its users are already prepared to handle that. > > Old behavior: > get_nprocs: > /sys/devices/system/cpu/online -> /proc/stat -> 2 > get_nprocs_conf: > /sys/devices/system/cpu/ -> /proc/stat -> 2 > > New behavior: > get_nprocs: > /sys/devices/system/cpu/online -> sched_getaffinity -> /proc/stat -> 2 > get_nprocs_conf: > /sys/devices/system/cpu/ -> /proc/stat -> sched_getaffinity -> 2 > > Fixes: 342298278e ("linux: Revert the use of sched_getaffinity on get_nproc") > Closes: BZ #28865 I think we are circling back on this, on BZ#27645 [1] we changed get_nprocs to use sched_getaffinity and then we have to revert it with BZ#28310 [2] because it introduced regression on some monitoring tools [3]. In fact from BZ#27645 and BZ#28624 [4] discussion I think we can't reliable use sched_getaffinity because since some container environment returns a synthetic mask that might break some programs. Also, sched_getaffinity returns a 'per-process' mask instead of system-wide as we discussed in previous threads. It should be ok to get adjusting internal tuning (as for malloc). [1] https://sourceware.org/bugzilla/show_bug.cgi?id=27645 [2] https://sourceware.org/bugzilla/show_bug.cgi?id=28310 [3] https://sourceware.org/bugzilla/show_bug.cgi?id=27645#c5 [4] https://sourceware.org/bugzilla/show_bug.cgi?id=28624 > --- > sysdeps/unix/sysv/linux/getsysstats.c | 101 ++++++++++++++++++-------- > 1 file changed, 70 insertions(+), 31 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c > index c98c8ce3d4..e1ed96070c 100644 > --- a/sysdeps/unix/sysv/linux/getsysstats.c > +++ b/sysdeps/unix/sysv/linux/getsysstats.c > @@ -50,9 +50,8 @@ __get_nprocs_sched (void) > is an arbitrary values assuming such systems should be rare and there > is no offline cpus. */ > return max_num_cpus; > - /* Some other error. 2 is conservative (not a uniprocessor system, so > - atomics are needed). */ > - return 2; > + /* Some other error. */ > + return 0; > } > > static char * > @@ -108,22 +107,19 @@ next_line (int fd, char *const buffer, char **cp, char **re, > } > > static int > -get_nproc_stat (char *buffer, size_t buffer_size) > +get_nproc_stat (void) > { > + enum { buffer_size = 1024 }; > + char buffer[buffer_size]; > char *buffer_end = buffer + buffer_size; > char *cp = buffer_end; > char *re = buffer_end; > - > - /* Default to an SMP system in case we cannot obtain an accurate > - number. */ > - int result = 2; > + int result = 0; > > const int flags = O_RDONLY | O_CLOEXEC; > int fd = __open_nocancel ("/proc/stat", flags); > if (fd != -1) > { > - result = 0; > - > char *l; > while ((l = next_line (fd, buffer, &cp, &re, buffer_end)) != NULL) > /* The current format of /proc/stat has all the cpu* entries > @@ -139,8 +135,8 @@ get_nproc_stat (char *buffer, size_t buffer_size) > return result; > } > > -int > -__get_nprocs (void) > +static int > +get_nprocs_cpu_online (void) > { > enum { buffer_size = 1024 }; > char buffer[buffer_size]; > @@ -179,7 +175,8 @@ __get_nprocs (void) > } > } > > - result += m - n + 1; > + if (m >= n) > + result += m - n + 1; > > l = endp; > if (l < re && *l == ',') > @@ -188,28 +185,18 @@ __get_nprocs (void) > while (l < re && *l != '\n'); > > __close_nocancel_nostatus (fd); > - > - if (result > 0) > - return result; > } > > - return get_nproc_stat (buffer, buffer_size); > + return result; > } > -libc_hidden_def (__get_nprocs) > -weak_alias (__get_nprocs, get_nprocs) > - > > -/* On some architectures it is possible to distinguish between configured > - and active cpus. */ > -int > -__get_nprocs_conf (void) > +static int > +get_nprocs_cpu (void) > { > - /* Try to use the sysfs filesystem. It has actual information about > - online processors. */ > + int count = 0; > DIR *dir = __opendir ("/sys/devices/system/cpu"); > if (dir != NULL) > { > - int count = 0; > struct dirent64 *d; > > while ((d = __readdir64 (dir)) != NULL) > @@ -224,12 +211,64 @@ __get_nprocs_conf (void) > > __closedir (dir); > > - return count; > } > + return count; > +} > > - enum { buffer_size = 1024 }; > - char buffer[buffer_size]; > - return get_nproc_stat (buffer, buffer_size); > +int > +__get_nprocs (void) > +{ > + int result; > + > + /* Try /sys/devices/system/cpu/online first. */ > + result = get_nprocs_cpu_online (); > + if (result) > + return result; > + > + /* Try sched_getaffinity(2). */ > + result = __get_nprocs_sched (); > + if (result) > + return result; > + > + /* Try /proc/stat. */ > + result = get_nproc_stat (); > + if (result) > + return result; > + > + /* We failed to obtain an accurate number. Be conservative: return > + the smallest number meaning that this is not a uniprocessor system, > + so atomics are needed. */ > + return 2; > +} > +libc_hidden_def (__get_nprocs) > +weak_alias (__get_nprocs, get_nprocs) > + > +/* On some architectures it is possible to distinguish between configured > + and active cpus. */ > +int > +__get_nprocs_conf (void) > +{ > + int result; > + > + /* Try /sys/devices/system/cpu/ first. */ > + result = get_nprocs_cpu (); > + if (result) > + return result; > + > + /* Try /proc/stat. */ > + result = get_nproc_stat (); > + if (result) > + return result; > + > + /* Try sched_getaffinity(2). */ > + result = __get_nprocs_sched (); > + if (result) > + return result; > + > + /* We failed to obtain an accurate number. Be conservative: return > + the smallest number meaning that this is not a uniprocessor system, > + so atomics are needed. */ > + return 2; > } > libc_hidden_def (__get_nprocs_conf) > weak_alias (__get_nprocs_conf, get_nprocs_conf) >