From patchwork Fri Nov 4 15:51:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 59944 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 207533858427 for ; Fri, 4 Nov 2022 15:52:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 207533858427 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1667577137; bh=9e7q1TJLUEAEWHftqHMjhKeMIbhHAoLIFWE/Ei5p9x0=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=cKnWIfRyDc3t+nBSxG95xcjZUcyFfZLzMlO/diR8eWDlfO6e6FlSszugrEeXlwxNm yPvx6By0w2lEjwlVLEWwYA8oex0HkBOEQiwDdPl4uzR/+9vhfdyJsiRkmdvWWL1OFs Wog2I06TsuR81kxySxJlRO7vRaRwJRAYNZ2SjLa8= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 89C3B3858403 for ; Fri, 4 Nov 2022 15:51:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 89C3B3858403 X-ASG-Debug-ID: 1667577099-0c856e167f290370001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id ZHZu532sxF8xwGzM (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Fri, 04 Nov 2022 11:51:39 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 189FC441B21; Fri, 4 Nov 2022 11:51:39 -0400 (EDT) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Subject: [PATCH 1/2] gdbsupport, gdb: add read_text_file_to_string, use it in linux_common_core_of_thread Date: Fri, 4 Nov 2022 11:51:36 -0400 X-ASG-Orig-Subj: [PATCH 1/2] gdbsupport, gdb: add read_text_file_to_string, use it in linux_common_core_of_thread Message-Id: <20221104155137.1463129-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.37.3 MIME-Version: 1.0 X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1667577099 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 4690 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.101904 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-3498.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Cc: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I would like to add more code to nat/linux-osdata.c that reads an entire file from /proc or /sys and processes it as a string afterwards. I would like to avoid duplicating the somewhat error-prone code that reads an entire file to a buffer. I think we should have a utility function that does that. Add read_file_to_string to gdbsupport/filestuff.{c,h}, and make linux_common_core_of_thread use it. I want to make the new function return an std::string, and because strtok doesn't play well with std::string (it requires a `char *`, std::string::c_str returns a `const char *`), change linux_common_core_of_thread to use std::string methods instead. Change-Id: I1793fda72a82969c28b944a84acb953f74c9230a --- gdb/nat/linux-osdata.c | 52 +++++++++++++++++------------------------ gdbsupport/filestuff.cc | 35 +++++++++++++++++++++++++++ gdbsupport/filestuff.h | 2 ++ 3 files changed, 58 insertions(+), 31 deletions(-) diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c index 3c31ee6fe50..f9c43f6691e 100644 --- a/gdb/nat/linux-osdata.c +++ b/gdb/nat/linux-osdata.c @@ -62,49 +62,39 @@ int linux_common_core_of_thread (ptid_t ptid) { char filename[sizeof ("/proc//task//stat") + 2 * MAX_PID_T_STRLEN]; - char *content = NULL; - char *p; - char *ts = 0; - int content_read = 0; - int i; int core; sprintf (filename, "/proc/%lld/task/%lld/stat", (PID_T) ptid.pid (), (PID_T) ptid.lwp ()); - gdb_file_up f = gdb_fopen_cloexec (filename, "r"); - if (!f) - return -1; - for (;;) - { - int n; - content = (char *) xrealloc (content, content_read + 1024); - n = fread (content + content_read, 1, 1024, f.get ()); - content_read += n; - if (n < 1024) - { - content[content_read] = '\0'; - break; - } - } + gdb::optional content = read_text_file_to_string (filename); + if (!content.has_value ()) + return -1; /* ps command also relies on no trailing fields ever contain ')'. */ - p = strrchr (content, ')'); - if (p != NULL) - p++; + std::string::size_type pos = content->find_last_of (')'); + if (pos == std::string::npos) + return -1; /* If the first field after program name has index 0, then core number is - the field with index 36. There's no constant for that anywhere. */ - if (p != NULL) - p = strtok_r (p, " ", &ts); - for (i = 0; p != NULL && i != 36; ++i) - p = strtok_r (NULL, " ", &ts); + the field with index 36 (so, the 37th). There's no constant for that + anywhere. */ + for (int i = 0; i < 37; ++i) + { + /* Find separator. */ + pos = content->find_first_of (' ', pos); + if (pos == std::string::npos) + return {}; + + /* Find beginning of field. */ + pos = content->find_first_not_of (' ', pos); + if (pos == std::string::npos) + return {}; + } - if (p == NULL || sscanf (p, "%d", &core) == 0) + if (sscanf (&(*content)[pos], "%d", &core) == 0) core = -1; - xfree (content); - return core; } diff --git a/gdbsupport/filestuff.cc b/gdbsupport/filestuff.cc index 2dfae5a48c5..d003a7f39c4 100644 --- a/gdbsupport/filestuff.cc +++ b/gdbsupport/filestuff.cc @@ -501,3 +501,38 @@ mkdir_recursive (const char *dir) component_start = component_end; } } + +gdb::optional +read_text_file_to_string (const char *path) +{ + gdb_file_up file = gdb_fopen_cloexec (path, "r"); + if (file == nullptr) + return {}; + + std::string res; + for (;;) + { + std::string::size_type start_size = res.size (); + constexpr int chunk_size = 1024; + + /* Resize to accomodate CHUNK_SIZE bytes. */ + res.resize (start_size + chunk_size); + + int n = fread (&res[start_size], 1, chunk_size, file.get ()); + if (n == chunk_size) + continue; + + gdb_assert (n < chunk_size); + + /* Less than CHUNK means EOF or error. If it's an error, return + no value. */ + if (ferror (file.get ())) + return {}; + + /* Resize the string according to the data we read. */ + res.resize (start_size + n); + break; + } + + return res; +} diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h index 4bc9249dcbf..3932710b5dc 100644 --- a/gdbsupport/filestuff.h +++ b/gdbsupport/filestuff.h @@ -129,4 +129,6 @@ extern bool is_regular_file (const char *name, int *errno_ptr); extern bool mkdir_recursive (const char *dir); +extern gdb::optional read_text_file_to_string (const char *path); + #endif /* COMMON_FILESTUFF_H */ From patchwork Fri Nov 4 15:51:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 59946 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B79423858038 for ; Fri, 4 Nov 2022 15:52:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B79423858038 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1667577169; bh=1InEuuM4ppIWIYCh175HaPV2z8G7tp9tZVlJMYYeXyY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=mlVmNQd0r/uYcycSrKemGZidvGPXrIl3fQrVxvrnYXpti7rdWKogTmiD+KxlN2j00 2R67ZtxcX/UzLcC46rzGLUCWte3q5VQskRXzKy4NwL47zQI9Maryfv9e9zEjoirVH0 BA6L6JoJ2DjdrwyZzpe6Lps1lf61jyZ6ERCcGE8g= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 4166F385840D for ; Fri, 4 Nov 2022 15:51:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4166F385840D X-ASG-Debug-ID: 1667577099-0c856e16802904a0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id UQx6X8y8E92tlsyW (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Fri, 04 Nov 2022 11:51:39 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 35617441D64; Fri, 4 Nov 2022 11:51:39 -0400 (EDT) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Subject: [PATCH 2/2] gdb/linux-nat: get core count using /sys/devices/system/cpu/possible Date: Fri, 4 Nov 2022 11:51:37 -0400 X-ASG-Orig-Subj: [PATCH 2/2] gdb/linux-nat: get core count using /sys/devices/system/cpu/possible Message-Id: <20221104155137.1463129-2-simon.marchi@efficios.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221104155137.1463129-1-simon.marchi@efficios.com> References: <20221104155137.1463129-1-simon.marchi@efficios.com> MIME-Version: 1.0 X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1667577099 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 5349 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.101904 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-3498.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Cc: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I get this test failure on my CI; FAIL: gdb.base/info-os.exp: get process list The particularity of this setup is that builds are done in containers who are allocated 4 CPUs on a machine that has 40. The code in nat/linux-osdata.c fails to properly fetch the core number for each task. linux_xfer_osdata_processes uses `sysconf (_SC_NPROCESSORS_ONLN)`, which returns 4, so it allocates an array of 4 integers. However, the core numbers read from /proc/pid/task/tid/stat, by function linux_common_core_of_thread, returns a value anywhere between 0 and 39. The core numbers above 3 are therefore ignored, many processes end up with no core value, and the regexp in the test doesn't match (it requires an integer as the core field). The way this the CPUs are exposed to the container is that the container sees 40 CPUs "present" and "possible", but only 4 arbitrary CPUs actually online: root@ci-node-jammy-amd64-04-08:~# cat /sys/devices/system/cpu/present 0-39 root@ci-node-jammy-amd64-04-08:~# cat /sys/devices/system/cpu/online 5,11,24,31 root@ci-node-jammy-amd64-04-08:~# cat /sys/devices/system/cpu/possible 0-39 The solution proposed in this patch is to find out the number of possible CPUs using /sys/devices/system/cpu/possible. In practice, this will probably always contain `0-N`, where N is the number of CPUs, minus one. But the documentation [1] doesn't such guarantee, so I'll assume that it can contain a more complex range list such as `2,4-31,32-63`, like the other files in that directory can have. The solution is to iterate over these numbers to find the highest possible CPU id, and use that that value plus one as the size of the array to allocate. [1] https://www.kernel.org/doc/Documentation/admin-guide/cputopology.rst Change-Id: I7abce2e43b000c1327fa94cd7b99d46e49d7ccf3 --- gdb/nat/linux-osdata.c | 70 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c index f9c43f6691e..8639f090910 100644 --- a/gdb/nat/linux-osdata.c +++ b/gdb/nat/linux-osdata.c @@ -271,6 +271,68 @@ get_cores_used_by_process (PID_T pid, int *cores, const int num_cores) return task_count; } +/* get_core_array_size helper that uses /sys/devices/system/cpu/possible. */ + +static gdb::optional +get_core_array_size_using_sys_possible () +{ + gdb::optional possible + = read_text_file_to_string ("/sys/devices/system/cpu/possible"); + + if (!possible.has_value ()) + return {}; + + /* The format is documented here: + + https://www.kernel.org/doc/Documentation/admin-guide/cputopology.rst + + For the purpose of this function, we assume the file can contain a complex + set of ranges, like `2,4-31,32-63`. Read all number, disregarding commands + and dashes, in order to find the largest possible core number. The size + of the array to allocate is that plus one. */ + + unsigned long max_id = 0; + for (std::string::size_type start = 0; start < possible->size ();) + { + const char *start_p = &(*possible)[start]; + char *end_p; + + /* Parse one number. */ + errno = 0; + unsigned long id = strtoul (start_p, &end_p, 10); + if (errno != 0) + return {}; + + max_id = std::max (max_id, id); + + start += end_p - start_p; + gdb_assert (start <= possible->size ()); + + /* Skip comma, dash, or new line (if we are at the end). */ + ++start; + } + + return max_id + 1; +} + +/* Return the array size to allocate in order to be able to index it using + CPU core numbers. This may be more than the actual number of cores if + the core numbers are not contiguous. */ + +static size_t +get_core_array_size () +{ + /* Using /sys/.../possible is prefered, because it handles the case where + we are in a container that has access to a subset of the host's cores. + It will return a size that considers all the CPU cores available to the + host. If that fials for some reason, fall back to sysconf. */ + gdb::optional count = get_core_array_size_using_sys_possible (); + if (count.has_value ()) + return *count; + + return sysconf (_SC_NPROCESSORS_ONLN); +} + static void linux_xfer_osdata_processes (struct buffer *buffer) { @@ -281,7 +343,7 @@ linux_xfer_osdata_processes (struct buffer *buffer) dirp = opendir ("/proc"); if (dirp) { - const int num_cores = sysconf (_SC_NPROCESSORS_ONLN); + const int core_array_size = get_core_array_size (); struct dirent *dp; while ((dp = readdir (dirp)) != NULL) @@ -308,10 +370,10 @@ linux_xfer_osdata_processes (struct buffer *buffer) strcpy (user, "?"); /* Find CPU cores used by the process. */ - cores = XCNEWVEC (int, num_cores); - task_count = get_cores_used_by_process (pid, cores, num_cores); + cores = XCNEWVEC (int, core_array_size); + task_count = get_cores_used_by_process (pid, cores, core_array_size); - for (i = 0; i < num_cores && task_count > 0; ++i) + for (i = 0; i < core_array_size && task_count > 0; ++i) if (cores[i]) { string_appendf (cores_str, "%d", i);