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 E6F49C3DA79 for ; Mon, 15 Jan 2024 06:59:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 74F0F6B007D; Mon, 15 Jan 2024 01:59:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6FD306B0080; Mon, 15 Jan 2024 01:59:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5C4726B0081; Mon, 15 Jan 2024 01:59:30 -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 472D86B007D for ; Mon, 15 Jan 2024 01:59:30 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0DA95160778 for ; Mon, 15 Jan 2024 06:59:30 +0000 (UTC) X-FDA: 81680644500.18.A851381 Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by imf03.hostedemail.com (Postfix) with ESMTP id 4FD1620008 for ; Mon, 15 Jan 2024 06:59:28 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=x4G0IDWO; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf03.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=1705301968; 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=X1mGZPNqR1VutiB4f8CGyXOdq7/LY5vNtMgTZcXL4nw=; b=W3IVn6/QdGgeOF4ZF5g0SF3EQNndNHHklJz7JcKCuoi7HW7/zD8lAgdmOJ8YRF0vy6nYQE KpcwKBTM9ozChH4LpXAGVar4Yz+wTfoQS/v7qocq7JizO7b3P9LHg+a4L+Slo6P0V3A4sC a6fho+vZHt4eb4hszwxZnTEzOA1zW9Q= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=x4G0IDWO; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf03.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=1705301968; a=rsa-sha256; cv=none; b=lRjHkTDau2b+hOzSbtUKKcKX8G0OtsBoWxw+DvN91chYrsPusVD2D8kDPYUCL6EjIDZnWX yxrk/BwQrEuECxSSKimwVHc7bxJBAAVJLdguUfYxA3HSWzSGcrMpKieeyTxw9iARWpuKrV 85fEyi0guX7/oqT+S9CwrieQZ6JSIRg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1705301966; bh=PUNiFql2LzYt4AfUj6+kFyjiKSLCP2jzF69eYRLV3m8=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=x4G0IDWOY9mEwa96umEJ9cNBDjBE4yobVBcd4b24K9J2SDz+Xbq91JY3wHcD3J0XY cxAEJCIb0PbJP2BRUiPQEJs4eIztkhtfEUAbFOT1ySKJTdmf186KS+TL6RT0TD3hDe XitrekO2fzeO27o9ECzEkxPKXgENniSs/eO5lUGceSw8JL1xyulBqytWZoce0ZRJKh eeZcMa8E9aCWCvLNd4dUETYLqhjVgSa6yQR6wAl541jhZIjTQf1aQLbgQfRdoWHl2S RjwXQxpYLfShFF79t0I+JbDNYqByetDJwhxoxx6/5so3WJSn06WGK15wAwgz9S6NNC EPOpkWzGzaqBw== 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 1CB6E3780C35; Mon, 15 Jan 2024 06:59:24 +0000 (UTC) Message-ID: Date: Mon, 15 Jan 2024 11:59:33 +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 Content-Language: en-US To: Jiaqi Yan References: <20240112072144.620098-1-usama.anjum@collabora.com> <20240112072144.620098-5-usama.anjum@collabora.com> <0b5a7c8c-d46b-496c-b705-fafc54a91ff0@collabora.com> From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4FD1620008 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: z4q884kg1s4pwm7m76xua9znxyhn7kid X-HE-Tag: 1705301968-194726 X-HE-Meta: U2FsdGVkX1/xRg8KKHAFDDy24pgUVbVWZGjDTon2BFzMzMccKwibxbl0t5kTtneoDa9OpGlxcOpCjOFt5J2oxXiXBeeNEt27DgVljOzF39qlmh+NvmE7tc+PpLkeMbArMA0Yi+AxAePMHiccDp2h0NUpwNakoqA6DR9Y20fIho+V+zAWrxmdOfoj1O7iKjZaYJAK8YI38rSEF/3qO4rgq/Srtn+ydj3ogHrgT6m3AU+s/f97RkCrS4ZFG5xMGvk6N9WdqXlxuXIjxjwuFhqRKYe998U3GQOl+oh8PdT8ywhwmMsKyitQtf06K94HkNsnDWo1VGK7sm3AiWHwKUbdHgGsEHNjO6WwkVcGQclmdFg80bGyJAzAfWBvVEl/6qV99eQFKq1QhKdMsoDR3MNwFycoVCVwYHdgyLKNsnVp12j+CRHrRQKHWjUUr0H0mSNjFrtTz1ffbkOOzKUieVnE7BMqY6Rl5di7OVNpa0N8orTU2uVitVmAusM7lUAJ4jZIm0IZoVdfBbLDDoyRA5qA9n6GndwMvmPxTOeQamC0RGXFoVEJQGGHLaStK3+gdTQAsJOTCPGTpZ935LSF5fwX/o9xEwyXl/If6fvrSzGmZqMcE/1yTdBImyWSdAIcSePc7/d+1RLiIxmuaOrnBOBHHOZ8lb+cCvv8871bUHry6mdk3CwMxqlz95tPpu/vdqBKNvtl3zjtrK2GeQ8QQ/ckMXHvl+9o5OLEy+os7WIrsSmZkPPtenZ/nX/KiOWd0AKPw1sZzUUaSuYdqz+ZqWiKUBtB/aH6vkohJqi60VlDDI9ytGzDG1xFhTT0NPSAp30oUDbEZc0t7wtra6oGM4tw2KPbae1uAmr8T2kzfmueJgOlUaty/hjks7K5ku8p2S8Xq/YQ9BEnglNb7E1MFFx6l3qJ4glZodinI5bM9HaT8jsGOM5cZdFfaC6hDQm3UIB9 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/15/24 11:52 AM, Jiaqi Yan wrote: > On Sun, Jan 14, 2024 at 10:32 PM Muhammad Usama Anjum > wrote: >> >> 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. > > can't you use this ksft_perror available today? > https://github.com/torvalds/linux/blob/master/tools/testing/selftests/kselftest.h#L161C25-L172 Sorry, I wan't able to find it. It has missing signature in header comment as well. I'll just update this current test. I'll use ksft_perror in future patches. Thanks for mentining it. > >> >>> >>>> >>>> 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 > -- BR, Muhammad Usama Anjum