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 7851FC3DA79 for ; Mon, 15 Jan 2024 06:33:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D06426B007B; Mon, 15 Jan 2024 01:33:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CB4B36B007D; Mon, 15 Jan 2024 01:33:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B55596B007E; Mon, 15 Jan 2024 01:33:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 9FFFD6B007B for ; Mon, 15 Jan 2024 01:33:02 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 6D3AB80490 for ; Mon, 15 Jan 2024 06:33:02 +0000 (UTC) X-FDA: 81680577804.07.9626892 Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by imf18.hostedemail.com (Postfix) with ESMTP id 738AE1C000A for ; Mon, 15 Jan 2024 06:33:00 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=nWtOjvFm; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf18.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705300380; a=rsa-sha256; cv=none; b=7wMGaVXHLDD/FgAiP6+GNk6eRwbOTHowD44jKVfsizYbgd1NyXCNXTDig0AGWvByrWKUUg IYNa8Jl4CNzwH8w/SW3qqMGT+9uffcQo5IM/SEl+8t77OopDH5sSLsGsqNT567Is84Qows jefDAQjhHvukVTldUzDq76cdohFIxsk= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=nWtOjvFm; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf18.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705300380; 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=QaYQ1KOy4l0Vlz2W0mwQW9GTX2SCDDwOlkWELDR6x94=; b=Hwigb4Fmtl8Wzf2GSMaaWkUpNpdg+grBzYAPhdFaeUs0YFz/Y/lUwrHh5GmlfdV3T0O/Zx zD5RGdVHvSOt/3p8OR28wnY3zD06ADTui6+NMVvtbsAgigV74W8VXVIGxB5TSJSbINIVFV OzhiemoQhXZrGueUWvTG7K6a5t8ujeU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1705300378; bh=NuaQAsj4Nv/M74on8DVkHfKgJllQgVJbG4MYVFBzfPU=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=nWtOjvFmjfOEdjmuT58bpOA8WY2kbSyLAI4Eca6+q6043qOavMZWI3xsssoYKDxHK rrA0C9KU4Lu4jcoqCxLDgHXr4VP765G1EA8Hip2jNdKHHUVXRhcb9xV7XWYMSQByiv HuJrJ7kHi5/Jncl8kAXa7ZWpr/YRdT2VumTjG6prvzJQd84e2LvKZgmPZ44WvtbL2Y xfTDJVCufGr00zWgH4BRxDWZ3zyQSdE/MfyAjFhRoOdK1maIFDaMqLQm6HHpGdXYpU NbwMge4iMgKFuiHe/f62KiIXbxUfHfAfRRL5tMKxbaLxuJ7Ozgqqefi4igEEewLvgV jy54u1+7Lbgqw== Received: from [100.96.234.34] (cola.collaboradmins.com [195.201.22.229]) (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 06C153780C35; Mon, 15 Jan 2024 06:32:56 +0000 (UTC) Message-ID: <0b5a7c8c-d46b-496c-b705-fafc54a91ff0@collabora.com> Date: Mon, 15 Jan 2024 11:33:06 +0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: Muhammad Usama Anjum , Andrew Morton , Shuah Khan , kernel@collabora.com, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 5/7] selftests/mm: hugetlb-read-hwpoison: conform test to TAP format output To: Jiaqi Yan References: <20240112072144.620098-1-usama.anjum@collabora.com> <20240112072144.620098-5-usama.anjum@collabora.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 738AE1C000A X-Stat-Signature: 4iy34ctun9jahjfaewtez5ycyxz15rsp X-HE-Tag: 1705300380-551485 X-HE-Meta: U2FsdGVkX1+J7f1BRrT9JDopgzuui7SAw1qZEE9Tc/TK36BiL4d0cHNgytlnoBWNwH96lUGEB7g5DrIph9mxsgVXiyNOOeuYn2CFYlxcgwjkeMVE+jCyDeWpXDVUNbJ+Lo6mvsYpQaad6l/yvX32fBotTG95ATfBMcR4SH5Js8UuJeMvYY07KhkRmcJ5oxifGja0Omo/5kRAe/wcCOfmBNknEKG7Goc5RijZS0YYO9xpmzilt62karYwg5WydOgAO/I7SpJb1hARNaJFGWX4doJxjbdgZPHY7nkP1C0pQUkyYFSVZ7129mobcOv5IKj5xJgSHumSFVtwnPRPGiKGxI+DgA/oZu2FIIywkvCg+ZZjSkzbgS5/zTlCqpNaNk7rh+4aYlHudFFH+yyJ+/OJVnsH+adSaUNYZHyTlrIn3/gCSSQs40Sw1j6ZZB98PP4Cm5AF9LNCRkLTcuUwAzslje/5Hv12GaLatIWzZLBAoR/u91AzL/3CPQOf1T5pTQlah1q3WhHFXyrxSExvjd/xyWVvtzcl1sWqx54pK3aGkj1UpjPdSc7LpW1ey9Y/sfHAFqaXFLO8zfA4h68ZyKhK2ejzKkgA8P4uT9YHDK0trF9PF8ZChtRohdoDCcl7SFIIc7Oanix87Pq7hvPd2nu+xyg7XrLDvGisbxwNWuXmNoXgmiNnUEKoBPPatGt1U/Itb896cpidP8gqmveDWR/KTOtSKNuZCIKKjOtnuXOOGDoi2SesghINPVx381xwet9k90CWkNuv5s1jU35DFe9CZsiLk+cHB+0ZJ4f/2u3Ft0nvLBo7f/xpKMBb0ytT3WZBKQBGU5WUcfYJinjG9XgAItlmEdtzEKsBLo5IvpkEdRfdBFo8wpVaYPS3QKFQpJErgG7y/Jo2S92DkKRCPN5mA+eLF7Dn80nkyH4W//AEy8wPfWpUfRDTQhLQXFMpQGHblX/u2ie/dOFDPRkC89Q DdifbLOR 9bJaoXSWjQoCF3u8jVR6GWOVvYmcVclf+ZZrnx92Bu239PkJnhbBh/u0cMemHUU+3Fu+NZEjByw2exCnH9IF3fiUZkabbUvZUYnQ5toUeGfIUiHrBIO6GkMcHxkWgn+M056l6al2J3YMRqEC9Biw/wnUQe/63Mrleflpj018TTo3c+mUGfrtbN6NZ1aGAx55QaaqAi02h1R73X+/WpkaVwYBAV/nwC9/2weK9Gf6rKS2K0UpZLYY7GZ07zQ2AOtqrzycFagA8ytsM66+DfUyVGbfn/2AFtUpWtIkSaOxXcTckelBbDvcS40QUqKeso+BmJK1pXq79vWMAfpxj1NCsS/HnqNIU53t7i863gN+itvg4rqM3gvVzhRCjIFptUnFN7lVe 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 1/13/24 6:08 AM, Jiaqi Yan wrote: > On Thu, Jan 11, 2024 at 11:21 PM 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 >> --- >> Tested this by reverting the patch a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3 >> ("mm/filemap: remove hugetlb special casing in filemap.c") as it has >> broken the test. The bug report can be found at [1]. >> >> Tested with proposed fix as well [2]. >> >> [1] https://lore.kernel.org/all/079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com >> [2] https://lore.kernel.org/all/a20e7bdb-7344-306d-e8f5-5ee69af7d5ea@oracle.com >> --- >> .../selftests/mm/hugetlb-read-hwpoison.c | 116 +++++++++--------- >> 1 file changed, 56 insertions(+), 60 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> index ba6cc6f9cabc..193ad7275df5 100644 >> --- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> +++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c >> @@ -58,8 +58,8 @@ static bool verify_chunk(char *buf, size_t len, char val) >> >> for (i = 0; i < len; ++i) { >> if (buf[i] != val) { >> - printf(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >> - i, buf[i], val); >> + ksft_print_msg(PREFIX ERROR_PREFIX "check fail: buf[%lu] = %u != %u\n", >> + i, buf[i], val); >> return false; >> } >> } >> @@ -75,9 +75,9 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> ssize_t total_ret_count = 0; >> char val = offset / wr_chunk_size + offset % wr_chunk_size; >> >> - printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> - expected); >> + ksft_print_msg(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset); >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> + expected); >> if (lseek(fd, offset, SEEK_SET) < 0) { >> perror(PREFIX ERROR_PREFIX "seek failed"); >> return false; >> @@ -86,7 +86,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> while (offset + total_ret_count < len) { >> ret_count = read(fd, buf, wr_chunk_size); >> if (ret_count == 0) { >> - printf(PREFIX PREFIX "read reach end of the file\n"); >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >> break; >> } else if (ret_count < 0) { >> perror(PREFIX ERROR_PREFIX "read failed"); >> @@ -98,8 +98,8 @@ static bool seek_read_hugepage_filemap(int fd, size_t len, size_t wr_chunk_size, >> >> total_ret_count += ret_count; >> } >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> - total_ret_count); >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> + total_ret_count); >> >> return total_ret_count == expected; >> } >> @@ -112,15 +112,15 @@ static bool read_hugepage_filemap(int fd, size_t len, >> ssize_t total_ret_count = 0; >> char val = 0; >> >> - printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> - expected); >> + ksft_print_msg(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n", >> + expected); >> while (total_ret_count < len) { >> ret_count = read(fd, buf, wr_chunk_size); >> if (ret_count == 0) { >> - printf(PREFIX PREFIX "read reach end of the file\n"); >> + ksft_print_msg(PREFIX PREFIX "read reach end of the file\n"); >> break; >> } else if (ret_count < 0) { >> - perror(PREFIX ERROR_PREFIX "read failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "read failed"); > > Should we also include strerror(errno) in log msg, like you did below? Looks like I missed it. I'll post a v3. > > Actually, would ksft_perror be a better choice for perror()s? It may or may not be. There aren't ksft_*_perror macros for printing the logs at this time. I prefer using just whatever macros are available. They aren't generic enough. Maybe mm tests use perror whenever error occurs, but other tests don't. > >> >> break; >> } >> ++val; >> @@ -129,8 +129,8 @@ static bool read_hugepage_filemap(int fd, size_t len, >> >> total_ret_count += ret_count; >> } >> - printf(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> - total_ret_count); >> + ksft_print_msg(PREFIX PREFIX "actually read 0x%lx bytes of data in total\n", >> + total_ret_count); >> >> return total_ret_count == expected; >> } >> @@ -142,14 +142,15 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >> char *filemap = NULL; >> >> if (ftruncate(fd, len) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >> return status; >> } >> >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >> MAP_SHARED | MAP_POPULATE, fd, 0); >> if (filemap == MAP_FAILED) { >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >> + strerror(errno)); >> goto done; >> } >> >> @@ -162,7 +163,8 @@ test_hugetlb_read(int fd, size_t len, size_t wr_chunk_size) >> munmap(filemap, len); >> done: >> if (ftruncate(fd, 0) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed : %s\n", >> + strerror(errno)); >> status = TEST_FAILED; >> } >> >> @@ -179,14 +181,15 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> const unsigned long pagesize = getpagesize(); >> >> if (ftruncate(fd, len) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate failed: %s\n", strerror(errno)); >> return status; >> } >> >> filemap = mmap(NULL, len, PROT_READ | PROT_WRITE, >> MAP_SHARED | MAP_POPULATE, fd, 0); >> if (filemap == MAP_FAILED) { >> - perror(PREFIX ERROR_PREFIX "mmap for primary mapping failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "mmap for primary mapping failed: %s\n", >> + strerror(errno)); >> goto done; >> } >> >> @@ -201,7 +204,7 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> */ >> hwp_addr = filemap + len / 2 + pagesize; >> if (madvise(hwp_addr, pagesize, MADV_HWPOISON) < 0) { >> - perror(PREFIX ERROR_PREFIX "MADV_HWPOISON failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "MADV_HWPOISON failed: %s\n", strerror(errno)); >> goto unmap; >> } >> >> @@ -228,7 +231,8 @@ test_hugetlb_read_hwpoison(int fd, size_t len, size_t wr_chunk_size, >> munmap(filemap, len); >> done: >> if (ftruncate(fd, 0) < 0) { >> - perror(PREFIX ERROR_PREFIX "ftruncate back to 0 failed"); >> + ksft_print_msg(PREFIX ERROR_PREFIX "ftruncate back to 0 failed: %s\n", >> + strerror(errno)); >> status = TEST_FAILED; >> } >> >> @@ -240,27 +244,32 @@ static int create_hugetlbfs_file(struct statfs *file_stat) >> int fd; >> >> fd = memfd_create("hugetlb_tmp", MFD_HUGETLB); >> - if (fd < 0) { >> - perror(PREFIX ERROR_PREFIX "could not open hugetlbfs file"); >> - return -1; >> - } >> + if (fd < 0) >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "could not open hugetlbfs file: %s\n", >> + strerror(errno)); >> >> memset(file_stat, 0, sizeof(*file_stat)); >> + >> if (fstatfs(fd, file_stat)) { >> - perror(PREFIX ERROR_PREFIX "fstatfs failed"); >> - goto close; >> + close(fd); >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "fstatfs failed: %s\n", strerror(errno)); >> } >> if (file_stat->f_type != HUGETLBFS_MAGIC) { >> - printf(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >> - goto close; >> + close(fd); >> + ksft_exit_fail_msg(PREFIX ERROR_PREFIX "not hugetlbfs file\n"); >> } >> >> return fd; >> -close: >> - close(fd); >> - return -1; >> } >> >> +#define KSFT_PRINT_MSG(status, fmt, ...) \ >> + do { \ >> + if (status == TEST_SKIPPED) \ >> + ksft_test_result_skip(fmt, __VA_ARGS__); \ >> + else \ >> + ksft_test_result(status == TEST_PASSED, fmt, __VA_ARGS__); \ >> + } while (0) >> + >> int main(void) >> { >> int fd; >> @@ -273,50 +282,37 @@ int main(void) >> }; >> size_t i; >> >> + ksft_print_header(); >> + ksft_set_plan(12); >> + >> for (i = 0; i < ARRAY_SIZE(wr_chunk_sizes); ++i) { >> - printf("Write/read chunk size=0x%lx\n", >> - wr_chunk_sizes[i]); >> + ksft_print_msg("Write/read chunk size=0x%lx\n", >> + wr_chunk_sizes[i]); >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB read regression test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB read regression test...\n"); >> status = test_hugetlb_read(fd, file_stat.f_bsize, >> wr_chunk_sizes[i]); >> - printf(PREFIX "HugeTLB read regression test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read regression test...%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB read HWPOISON test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB read HWPOISON test...\n"); >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >> wr_chunk_sizes[i], false); >> - printf(PREFIX "HugeTLB read HWPOISON test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB read HWPOISON test..%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> >> fd = create_hugetlbfs_file(&file_stat); >> - if (fd < 0) >> - goto create_failure; >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >> + ksft_print_msg(PREFIX "HugeTLB seek then read HWPOISON test...\n"); >> status = test_hugetlb_read_hwpoison(fd, file_stat.f_bsize, >> wr_chunk_sizes[i], true); >> - printf(PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >> - status_to_str(status)); >> + KSFT_PRINT_MSG(status, PREFIX "HugeTLB seek then read HWPOISON test...%s\n", >> + status_to_str(status)); >> close(fd); >> - if (status == TEST_FAILED) >> - return -1; >> } >> >> - return 0; >> - >> -create_failure: >> - printf(ERROR_PREFIX "Abort test: failed to create hugetlbfs file\n"); >> - return -1; >> + ksft_finished(); >> } >> -- >> 2.42.0 >> >> > -- BR, Muhammad Usama Anjum