From patchwork Fri Sep 29 08:21:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Maciej Wieczor-Retman X-Patchwork-Id: 13403813 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BDB7E743F6 for ; Fri, 29 Sep 2023 08:31:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232798AbjI2Ib3 (ORCPT ); Fri, 29 Sep 2023 04:31:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46488 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232663AbjI2Ib3 (ORCPT ); Fri, 29 Sep 2023 04:31:29 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02C8C1A7; Fri, 29 Sep 2023 01:31:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695976287; x=1727512287; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=E6Kx4UtEU9DgP25ZBND/clnbDPg65fa2ZP8121QiGAc=; b=Lb5sO2K3Tb+IcM5I0OycfFUGje+Zu1QMFFCWx/xaL0WN1kU2iSfNq+2F s5y9nc+N6wLbebMVY3YIbdOoe4FSOo2cfhn/pW4DTmUx5RH5MSSZhw6Tg lG2QhqikRk7joWSLuk79zmQNU1PoKbZWTLaSCZGZcDuFCga9i9IIz1mVN VCjyvcZX9da7/4CYmEaV8BpsVM5eyK1+CxANfhF0F8TiCPJIZ8kNlIcok 7dclH66rEkGGZAHX/cbWNWWjH5vSXsiEZAKgRsYGtVF0UuMION5fjVrUv nrU/HSRPF1MGO3kSgjpCbWNVzyO6r0ajFwKvrEawcCzBVwjDWXBR9QEVE Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="381144371" X-IronPort-AV: E=Sophos;i="6.03,186,1694761200"; d="scan'208";a="381144371" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2023 01:22:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="865622555" X-IronPort-AV: E=Sophos;i="6.03,186,1694761200"; d="scan'208";a="865622555" Received: from tzebrows-mobl.ger.corp.intel.com (HELO wieczorr-mobl1.intel.com) ([10.213.26.85]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2023 01:22:10 -0700 From: Maciej Wieczor-Retman To: Fenghua Yu , Reinette Chatre , Shuah Khan Cc: ilpo.jarvinen@linux.intel.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 1/2] selftests/resctrl: Fix schemata write error check Date: Fri, 29 Sep 2023 10:21:41 +0200 Message-ID: X-Mailer: git-send-email 2.42.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Writing bitmasks to the schemata can fail when the bitmask doesn't adhere to constraints defined by what a particular CPU supports. Some example of constraints are max length or having contiguous bits. The driver should properly return errors when any rule concerning bitmask format is broken. Resctrl FS returns error codes from fprintf() only when fclose() is called. Current error checking scheme allows invalid bitmasks to be written into schemata file and the selftest doesn't notice because the fclose() error code isn't checked. Substitute fopen(), flose() and fprintf() with open(), close() and write() to avoid error code buffering between fprintf() and fclose(). Remove newline character from the schema string after writing it to the schemata file so it prints correctly before function return. Pass the string generated with strerror() to the "reason" buffer so the error message is more verbose. Extend "reason" buffer so it can hold longer messages. Reviewed-by: Ilpo Järvinen Signed-off-by: Maciej Wieczor-Retman --- Changelog v5: - Add Ilpo's reviewed-by tag. - Fix wrong open() error checking. (Reinette) - Add error checking to schema_len variable. Changelog v4: - Unify error checking between open() and write(). (Reinette) - Add fcntl.h for glibc backward compatiblitiy. (Reinette) Changelog v3: - Rename fp to fd. (Ilpo) - Remove strlen, strcspn and just use the snprintf value instead. (Ilpo) Changelog v2: - Rewrite patch message. - Double "reason" buffer size to fit longer error explanation. - Redo file interactions with syscalls instead of stdio functions. tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++-------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 3a8111362d26..342a3dbcdbb6 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -8,6 +8,7 @@ * Sai Praneeth Prakhya , * Fenghua Yu */ +#include #include #include "resctrl.h" @@ -490,9 +491,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, */ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) { - char controlgroup[1024], schema[1024], reason[64]; - int resource_id, ret = 0; - FILE *fp; + char controlgroup[1024], schema[1024], reason[128]; + int resource_id, fd, schema_len = 0, ret = 0; if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) && strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) && @@ -520,27 +520,37 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) || !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) - sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata); + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n", + "L3:", resource_id, '=', schemata); if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) || !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) - sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata); + schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n", + "MB:", resource_id, '=', schemata); + if (schema_len < 1) { + snprintf(reason, sizeof(reason), + "snprintf() failed with return value : %d", schema_len); + ret = -1; + goto out; + } - fp = fopen(controlgroup, "w"); - if (!fp) { - sprintf(reason, "Failed to open control group"); + fd = open(controlgroup, O_WRONLY); + if (fd < 0) { + snprintf(reason, sizeof(reason), + "open() failed : %s", strerror(errno)); ret = -1; goto out; } - - if (fprintf(fp, "%s\n", schema) < 0) { - sprintf(reason, "Failed to write schemata in control group"); - fclose(fp); + if (write(fd, schema, schema_len) < 0) { + snprintf(reason, sizeof(reason), + "write() failed : %s", strerror(errno)); + close(fd); ret = -1; goto out; } - fclose(fp); + close(fd); + schema[schema_len - 1] = 0; out: ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n", From patchwork Fri Sep 29 08:21:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Maciej Wieczor-Retman X-Patchwork-Id: 13403814 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22426E743F8 for ; Fri, 29 Sep 2023 08:31:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232846AbjI2Ibb (ORCPT ); Fri, 29 Sep 2023 04:31:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231774AbjI2Iba (ORCPT ); Fri, 29 Sep 2023 04:31:30 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9D1261AE; Fri, 29 Sep 2023 01:31:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695976288; x=1727512288; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=PpM3zZkxJD1zLq2YlRs46MrA+Nb1yQ8pa7Hs4HrAios=; b=GeOnHA1FE35gvTp4GGmsjdzz99oJbnXAU23GDLAE+7g44cKSmgsBe2zx CK2k7rYlNQvJOnxN82E+QaspMtiAAsLYSaiahy+Cf+bqysoRk3cgqUTOY qcUu+hExeX5/tuPnZnBXeHGjONCB1LFBf8TN10ie3sPBa8nhA5g4hrKyg Dti6JGu8bPOOWUKQKN6g5RQO5TsHAitz2Lj8idJwUfDWlGrBiFL1iFkyh 50P4Gt8VCjViXq9e8AXmMc1vpJ47Z8qdYKAFl6/Ty92F+qvWftCStSfh5 VcC8Xr8yWklF+14NORYp+jgDKTgaj1hr4lGMBZYC0FrrxnVq++tl4pQ1X Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="381144408" X-IronPort-AV: E=Sophos;i="6.03,186,1694761200"; d="scan'208";a="381144408" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2023 01:22:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="865622705" X-IronPort-AV: E=Sophos;i="6.03,186,1694761200"; d="scan'208";a="865622705" Received: from tzebrows-mobl.ger.corp.intel.com (HELO wieczorr-mobl1.intel.com) ([10.213.26.85]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2023 01:22:20 -0700 From: Maciej Wieczor-Retman To: Fenghua Yu , Reinette Chatre , Shuah Khan Cc: ilpo.jarvinen@linux.intel.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 2/2] selftests/resctrl: Move run_benchmark() to a more fitting file Date: Fri, 29 Sep 2023 10:21:42 +0200 Message-ID: X-Mailer: git-send-email 2.42.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org resctrlfs.c contains mostly functions that interact in some way with resctrl FS entries while functions inside resctrl_val.c deal with measurements and benchmarking. run_benchmark() is located in resctrlfs.c even though it's purpose is not interacting with the resctrl FS but to execute cache checking logic. Move run_benchmark() to resctrl_val.c just before resctrl_val() that makes use of run_benchmark(). Make run_benchmark() static since it's not used between multiple files anymore. Remove return comment from kernel-doc since the function is type void. Reviewed-by: Ilpo Järvinen Reviewed-by: Reinette Chatre Signed-off-by: Maciej Wieczor-Retman --- Changelog v4: - Reword patch message very slightly. (Reinette) Changelog v3: - Make run_benchmark() static and remove it from the header. (Reinette) - Remove return void kernel-doc comment. (Ilpo) - Added Ilpo's reviewed-by tag. tools/testing/selftests/resctrl/resctrl.h | 1 - tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++ tools/testing/selftests/resctrl/resctrlfs.c | 52 ------------------- 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 8578a8b4e145..a33f414f6019 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -86,7 +86,6 @@ int validate_bw_report_request(char *bw_report); bool validate_resctrl_feature_request(const char *resource, const char *feature); char *fgrep(FILE *inf, const char *str); int taskset_benchmark(pid_t bm_pid, int cpu_no); -void run_benchmark(int signum, siginfo_t *info, void *ucontext); int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val); int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index a9fe61133119..0577e983067a 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -625,6 +625,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) return 0; } +/* + * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) + * in specified signal. Direct benchmark stdio to /dev/null. + * @signum: signal number + * @info: signal info + * @ucontext: user context in signal handling + */ +static void run_benchmark(int signum, siginfo_t *info, void *ucontext) +{ + int operation, ret, memflush; + char **benchmark_cmd; + size_t span; + bool once; + FILE *fp; + + benchmark_cmd = info->si_ptr; + + /* + * Direct stdio of child to /dev/null, so that only parent writes to + * stdio (console) + */ + fp = freopen("/dev/null", "w", stdout); + if (!fp) + PARENT_EXIT("Unable to direct benchmark status to /dev/null"); + + if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { + /* Execute default fill_buf benchmark */ + span = strtoul(benchmark_cmd[1], NULL, 10); + memflush = atoi(benchmark_cmd[2]); + operation = atoi(benchmark_cmd[3]); + if (!strcmp(benchmark_cmd[4], "true")) + once = true; + else if (!strcmp(benchmark_cmd[4], "false")) + once = false; + else + PARENT_EXIT("Invalid once parameter"); + + if (run_fill_buf(span, memflush, operation, once)) + fprintf(stderr, "Error in running fill buffer\n"); + } else { + /* Execute specified benchmark */ + ret = execvp(benchmark_cmd[0], benchmark_cmd); + if (ret) + perror("wrong\n"); + } + + fclose(stdout); + PARENT_EXIT("Unable to run specified benchmark"); +} + /* * resctrl_val: execute benchmark and measure memory bandwidth on * the benchmark diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 342a3dbcdbb6..385983b5c9ce 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -294,58 +294,6 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) return 0; } -/* - * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) - * in specified signal. Direct benchmark stdio to /dev/null. - * @signum: signal number - * @info: signal info - * @ucontext: user context in signal handling - * - * Return: void - */ -void run_benchmark(int signum, siginfo_t *info, void *ucontext) -{ - int operation, ret, memflush; - char **benchmark_cmd; - size_t span; - bool once; - FILE *fp; - - benchmark_cmd = info->si_ptr; - - /* - * Direct stdio of child to /dev/null, so that only parent writes to - * stdio (console) - */ - fp = freopen("/dev/null", "w", stdout); - if (!fp) - PARENT_EXIT("Unable to direct benchmark status to /dev/null"); - - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { - /* Execute default fill_buf benchmark */ - span = strtoul(benchmark_cmd[1], NULL, 10); - memflush = atoi(benchmark_cmd[2]); - operation = atoi(benchmark_cmd[3]); - if (!strcmp(benchmark_cmd[4], "true")) - once = true; - else if (!strcmp(benchmark_cmd[4], "false")) - once = false; - else - PARENT_EXIT("Invalid once parameter"); - - if (run_fill_buf(span, memflush, operation, once)) - fprintf(stderr, "Error in running fill buffer\n"); - } else { - /* Execute specified benchmark */ - ret = execvp(benchmark_cmd[0], benchmark_cmd); - if (ret) - perror("wrong\n"); - } - - fclose(stdout); - PARENT_EXIT("Unable to run specified benchmark"); -} - /* * create_grp - Create a group only if one doesn't exist * @grp_name: Name of the group