From mboxrd@z Thu Jan 1 00:00:00 1970 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA861C4829E for ; Thu, 15 Feb 2024 04:59:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 175788D0006; Wed, 14 Feb 2024 23:59:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0D7478D0001; Wed, 14 Feb 2024 23:59:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EBA608D0006; Wed, 14 Feb 2024 23:59:41 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id D81A18D0001 for ; Wed, 14 Feb 2024 23:59:41 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 73A2DA255B for ; Thu, 15 Feb 2024 04:59:41 +0000 (UTC) X-FDA: 81792835362.08.9353BA0 Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by imf12.hostedemail.com (Postfix) with ESMTP id A09C24000E for ; Thu, 15 Feb 2024 04:59:39 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=icCKSMLN; spf=pass (imf12.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=quarantine) header.from=collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707973180; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tYmf5SxuRH0Ym6JwxFMwBJ3IG72tIP2eUNLq7mlCnWY=; b=dcJ6xhaOhbMJrPVbmJpHXQhJbg2uPXQ6pAM3VcArUOBHBPzwX/W+5+bW+ifJZTrhphVmTx gIQ/mDSLLx/pXgjW9ZGuqfulxSPLIE1BF3sAmpl1pkDSoNht6w5Gb31kll69j7yW1QGgWN wRQZHj/omuy79Crc1kQZY8QIzN7OnRM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707973180; a=rsa-sha256; cv=none; b=IETGmCjKLeJIR5YRQq7C/B+Wai/52RhmmhkoSSwTlCV3Jftx1YJ/0ZYDOt0rHVm8j7Gl7h lBhTDPoIdDG1TZKxNcedotkkITni1qz+g4uHSlvlvML/rCp+rrZBGBQHeiA4MGR+iTsCd/ y/zy5N3D3g395LH6DU8W+R1ge9rTD+s= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=icCKSMLN; spf=pass (imf12.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=quarantine) header.from=collabora.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1707973177; bh=ajjEUWVtyZstjev69EjnPsEhJL0coDJnkzbbiToDta8=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=icCKSMLNJhYDjIw4Vf7gRpIEPxdm2fDcSvSxw3RiMCTGRp3gagsLsGlaVTG7rlYb2 H2c06lVd/PMdUI7rgx0WVdfg/gSnse0rKjR5CtvSQi1M7GMxt5Gg1XeTZ1xq4hNaSn YQHFdlugZDGmVhhyKwWv6Z2UPShf9yNxdfRL4H/poMnoWxgla1+2jhmCGgOaAu//Hx KZG+hgqevMkskq38NYg3HrXbvMwS08FxgMQ4qgyljDosowKLIpw5OS0X0rB2gJNPJV 6bjhTL1pizQetreiC13d0c9yU5xOTv2sO2roWLYj7cu7Lt1E6aYQanMZ+M4Ihl3t3h fj0cD0GNDhO0Q== Received: from [100.89.250.244] (ec2-34-240-57-77.eu-west-1.compute.amazonaws.com [34.240.57.77]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: usama.anjum) by madrid.collaboradmins.com (Postfix) with ESMTPSA id BA63137820A7; Thu, 15 Feb 2024 04:59:35 +0000 (UTC) Message-ID: Date: Thu, 15 Feb 2024 09:59:58 +0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: Muhammad Usama Anjum , kernel@collabora.com, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Aishwarya TCV Subject: Re: [PATCH v2 09/12] selftests/mm: thp_settings: conform to TAP format output To: Ryan Roberts , Andrew Morton , Shuah Khan References: <20240202113119.2047740-1-usama.anjum@collabora.com> <20240202113119.2047740-10-usama.anjum@collabora.com> <0c3182ae-885c-4156-980b-e35d825fe72e@arm.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: <0c3182ae-885c-4156-980b-e35d825fe72e@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: A09C24000E X-Rspam-User: X-Stat-Signature: uur9famxnii4ngc8w446htutd9575h6p X-Rspamd-Server: rspam03 X-HE-Tag: 1707973179-323201 X-HE-Meta: U2FsdGVkX188bXC7bZat2868YLIzVWFxLsteC3pLJrcx1NGQnOn73Ok2+4n3PEVVFcMiXnxcmZEwUxVQSN8odHZBVeWyBUErerw3fSvmgH4/k/Ui2LOB+fh1MdW2eQWOkmka1cyiG+tYJVUFon7x87YGyMCAJUC+kW6KwZTIAlhxJTyDIbndnSJ0SCG0BaYrObpJ3X3cUsnpXCVGdM/6UEYejgLrbe67+O4huKiJjIM9/2iL+1piT5JtbqKwR3UYp82Zf0kkSSggK1nrbN5GztZIHO4YH4GQlzbg7Hn06cKpM2WiRsOomGqVe0lhZCvSwTr7P3QfgK8Jbr/QbyoO4TlNcVGs3KrI1UFpryONyzFDxWB+7Q8VYjuxinsXRBwiwAXf7/c5iQoBhR/NhUOBMbhmpLN+i+Ft9JAyuWUMvIGqQ2A0M1wO5qQctnfaxOAbqnbbPZNWaNgy3nMFWYxMNI9mn6CEcUJZONxH+A5Fw26m+2SO7K2sXY5p8jGJSbrgvP7NscPJ0+LpU+XeIJy7pjsacPbbHgelAIB64I64mDgiEB7nYCSkgoGSU/6mCcvncw20yVg3cRVFOk72Yo71zFnuqzq+O8lG+CSE3a7an/84g4GCv4VZa6MqWzRFU2j0YKGFvXxoyiztWBGRIToLoC/qtCXG0kXfDmT18lNnislbNbHtnpAFjg3eYrv1mBgADzS37EAXjgmWcqnMUylkC9CqQ81/jJUEtuLzQWuzxCCmfB/rIZaDFculP4sUiLUW+1c418I6rcFD3imCBJ7zew3o/DrMeLJcxnUAdPq5TUlhPKudLOuxdfhbfHGTT7e05j10SauSJfRk0e75Gpne4iGYynytJvlq+kyhJJ1R7UdL+c2WlEjL3YQugXjXmdWHNfo2auK7p6V/FDEB0yM8NfwFNbKE6JNE0m+azuVUk5+SxyH3cyFthh57AhSNbTMXLayR1zAkovScye37CTO O2NtPJE5 6uvzyC+q5miz+TlKoXHxw2vMG3kK4RbSadaoMBKtSVmQxYq0m22gJeLCrCQ7TB8swIQvoU/llcfxJXPMTc58MtaS1BEtevd9QKXdbtACjASzU6lQ= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2/14/24 10:19 PM, Ryan Roberts wrote: > On 02/02/2024 11:31, Muhammad Usama Anjum wrote: >> Conform the layout, informational and status messages to TAP. No >> functional change is intended other than the layout of output messages. >> >> Signed-off-by: Muhammad Usama Anjum >> --- >> tools/testing/selftests/mm/khugepaged.c | 3 +- >> tools/testing/selftests/mm/thp_settings.c | 123 ++++++++-------------- >> tools/testing/selftests/mm/thp_settings.h | 4 +- >> 3 files changed, 47 insertions(+), 83 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c >> index d51fdaee7dc6a..3f202da0867c5 100644 >> --- a/tools/testing/selftests/mm/khugepaged.c >> +++ b/tools/testing/selftests/mm/khugepaged.c >> @@ -152,8 +152,7 @@ static void get_finfo(const char *dir) >> major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path)) >> ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> >> - if (read_file(path, buf, sizeof(buf)) < 0) >> - ksft_exit_fail_msg("read_file(read_num): %s\n", strerror(errno)); >> + read_file(path, buf, sizeof(buf)); >> >> if (strstr(buf, "DEVTYPE=disk")) { >> /* Found it */ >> diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c >> index a4163438108ec..273a95d025285 100644 >> --- a/tools/testing/selftests/mm/thp_settings.c >> +++ b/tools/testing/selftests/mm/thp_settings.c >> @@ -5,7 +5,9 @@ >> #include >> #include >> #include >> +#include >> >> +#include "../kselftest.h" >> #include "thp_settings.h" >> >> #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/" >> @@ -42,58 +44,45 @@ static const char * const shmem_enabled_strings[] = { >> NULL >> }; >> >> -int read_file(const char *path, char *buf, size_t buflen) >> +void read_file(const char *path, char *buf, size_t buflen) >> { >> int fd; >> ssize_t numread; >> >> fd = open(path, O_RDONLY); >> if (fd == -1) >> - return 0; >> + ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno)); > > Hi, > > This change has broken back compat. It's no longer possible to run cow and > khugepaged tests against older kernels. > > This function, as well as others in this file are intended to return 0 to > indicate the file could not be accessed (e.g. doesn't exist or don't have > permission, etc). Then higher level code can decide how to handle that. For > example, thp_supported_orders() determines which THP orders are supported by the > system based on the existence of certain files in sysfs. Then cow decides which > test variants to run based on the supported orders. With your change, it all > goes bang on the first probe and the whole test suite gets failed without > running any tests. > > I've no problem with improving the TAP output from tests, but this must only be > done at the test level, where it makes sense to do so. You can just call > ksft_exit_fail_msg() from deep within a utility function. > > Please can we remove this from mm-unstable. Sorry, not sure how I missed this. Let's drop this patch entirely. > > Thanks, > Ryan > > >> >> numread = read(fd, buf, buflen - 1); >> if (numread < 1) { >> close(fd); >> - return 0; >> + ksft_exit_fail_msg("No data read\n"); >> } >> >> buf[numread] = '\0'; >> close(fd); >> - >> - return (unsigned int) numread; >> } >> >> -int write_file(const char *path, const char *buf, size_t buflen) >> +void write_file(const char *path, const char *buf, size_t buflen) >> { >> int fd; >> ssize_t numwritten; >> >> fd = open(path, O_WRONLY); >> - if (fd == -1) { >> - printf("open(%s)\n", path); >> - exit(EXIT_FAILURE); >> - return 0; >> - } >> + if (fd == -1) >> + ksft_exit_fail_msg("%s open failed\n", path); >> >> numwritten = write(fd, buf, buflen - 1); >> close(fd); >> - if (numwritten < 1) { >> - printf("write(%s)\n", buf); >> - exit(EXIT_FAILURE); >> - return 0; >> - } >> - >> - return (unsigned int) numwritten; >> + if (numwritten < 1) >> + ksft_exit_fail_msg("write failed (%s)\n", buf); >> } >> >> const unsigned long read_num(const char *path) >> { >> char buf[21]; >> >> - if (read_file(path, buf, sizeof(buf)) < 0) { >> - perror("read_file()"); >> - exit(EXIT_FAILURE); >> - } >> + read_file(path, buf, sizeof(buf)); >> >> return strtoul(buf, NULL, 10); >> } >> @@ -103,10 +92,7 @@ void write_num(const char *path, unsigned long num) >> char buf[21]; >> >> sprintf(buf, "%ld", num); >> - if (!write_file(path, buf, strlen(buf) + 1)) { >> - perror(path); >> - exit(EXIT_FAILURE); >> - } >> + write_file(path, buf, strlen(buf) + 1); >> } >> >> int thp_read_string(const char *name, const char * const strings[]) >> @@ -117,30 +103,22 @@ int thp_read_string(const char *name, const char * const strings[]) >> int ret; >> >> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name); >> - if (ret >= PATH_MAX) { >> - printf("%s: Pathname is too long\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (ret >= PATH_MAX) >> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> >> - if (!read_file(path, buf, sizeof(buf))) { >> - perror(path); >> - exit(EXIT_FAILURE); >> - } >> + read_file(path, buf, sizeof(buf)); >> >> c = strchr(buf, '['); >> - if (!c) { >> - printf("%s: Parse failure\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (!c) >> + ksft_exit_fail_msg("%s: Parse failure\n", __func__); >> >> c++; >> memmove(buf, c, sizeof(buf) - (c - buf)); >> >> c = strchr(buf, ']'); >> - if (!c) { >> - printf("%s: Parse failure\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (!c) >> + ksft_exit_fail_msg("%s: Parse failure\n", __func__); >> + >> *c = '\0'; >> >> ret = 0; >> @@ -150,8 +128,8 @@ int thp_read_string(const char *name, const char * const strings[]) >> ret++; >> } >> >> - printf("Failed to parse %s\n", name); >> - exit(EXIT_FAILURE); >> + ksft_exit_fail_msg("Failed to parse %s\n", name); >> + return -1; >> } >> >> void thp_write_string(const char *name, const char *val) >> @@ -160,15 +138,10 @@ void thp_write_string(const char *name, const char *val) >> int ret; >> >> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name); >> - if (ret >= PATH_MAX) { >> - printf("%s: Pathname is too long\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (ret >= PATH_MAX) >> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> >> - if (!write_file(path, val, strlen(val) + 1)) { >> - perror(path); >> - exit(EXIT_FAILURE); >> - } >> + write_file(path, val, strlen(val) + 1); >> } >> >> const unsigned long thp_read_num(const char *name) >> @@ -177,10 +150,9 @@ const unsigned long thp_read_num(const char *name) >> int ret; >> >> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name); >> - if (ret >= PATH_MAX) { >> - printf("%s: Pathname is too long\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (ret >= PATH_MAX) >> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> + >> return read_num(path); >> } >> >> @@ -190,10 +162,9 @@ void thp_write_num(const char *name, unsigned long num) >> int ret; >> >> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name); >> - if (ret >= PATH_MAX) { >> - printf("%s: Pathname is too long\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (ret >= PATH_MAX) >> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> + >> write_num(path, num); >> } >> >> @@ -275,29 +246,26 @@ void thp_write_settings(struct thp_settings *settings) >> >> struct thp_settings *thp_current_settings(void) >> { >> - if (!settings_index) { >> - printf("Fail: No settings set"); >> - exit(EXIT_FAILURE); >> - } >> + if (!settings_index) >> + ksft_exit_fail_msg("Fail: No settings set\n"); >> + >> return settings_stack + settings_index - 1; >> } >> >> void thp_push_settings(struct thp_settings *settings) >> { >> - if (settings_index >= MAX_SETTINGS_DEPTH) { >> - printf("Fail: Settings stack exceeded"); >> - exit(EXIT_FAILURE); >> - } >> + if (settings_index >= MAX_SETTINGS_DEPTH) >> + ksft_exit_fail_msg("Fail: Settings stack exceeded\n"); >> + >> settings_stack[settings_index++] = *settings; >> thp_write_settings(thp_current_settings()); >> } >> >> void thp_pop_settings(void) >> { >> - if (settings_index <= 0) { >> - printf("Fail: Settings stack empty"); >> - exit(EXIT_FAILURE); >> - } >> + if (settings_index <= 0) >> + ksft_exit_fail_msg("Fail: Settings stack empty\n"); >> + >> --settings_index; >> thp_write_settings(thp_current_settings()); >> } >> @@ -335,14 +303,11 @@ unsigned long thp_supported_orders(void) >> for (i = 0; i < NR_ORDERS; i++) { >> ret = snprintf(path, PATH_MAX, THP_SYSFS "hugepages-%ukB/enabled", >> (getpagesize() >> 10) << i); >> - if (ret >= PATH_MAX) { >> - printf("%s: Pathname is too long\n", __func__); >> - exit(EXIT_FAILURE); >> - } >> + if (ret >= PATH_MAX) >> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); >> >> - ret = read_file(path, buf, sizeof(buf)); >> - if (ret) >> - orders |= 1UL << i; >> + read_file(path, buf, sizeof(buf)); >> + orders |= 1UL << i; >> } >> >> return orders; >> diff --git a/tools/testing/selftests/mm/thp_settings.h b/tools/testing/selftests/mm/thp_settings.h >> index 71cbff05f4c7f..04a6a7bbd08f8 100644 >> --- a/tools/testing/selftests/mm/thp_settings.h >> +++ b/tools/testing/selftests/mm/thp_settings.h >> @@ -56,8 +56,8 @@ struct thp_settings { >> struct hugepages_settings hugepages[NR_ORDERS]; >> }; >> >> -int read_file(const char *path, char *buf, size_t buflen); >> -int write_file(const char *path, const char *buf, size_t buflen); >> +void read_file(const char *path, char *buf, size_t buflen); >> +void write_file(const char *path, const char *buf, size_t buflen); >> const unsigned long read_num(const char *path); >> void write_num(const char *path, unsigned long num); >> > > -- BR, Muhammad Usama Anjum