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 C6F32C433EF for ; Wed, 16 Mar 2022 17:36:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D72196B0071; Wed, 16 Mar 2022 13:36:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D20DA8D0002; Wed, 16 Mar 2022 13:36:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE85F8D0001; Wed, 16 Mar 2022 13:36:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0021.hostedemail.com [216.40.44.21]) by kanga.kvack.org (Postfix) with ESMTP id AC5536B0071 for ; Wed, 16 Mar 2022 13:36:24 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 602809EB04 for ; Wed, 16 Mar 2022 17:36:24 +0000 (UTC) X-FDA: 79250953488.21.FDA8E28 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by imf16.hostedemail.com (Postfix) with ESMTP id 954D4180012 for ; Wed, 16 Mar 2022 17:36:23 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: usama.anjum) with ESMTPSA id 6F9591F430DD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1647452182; bh=CcIxudwQJMgXuoIKm9oLlx5LdLPJZ9BlYBuhMcMRnmI=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=IiPUZ/OADUJlHDKAwqADefd9ggOCIa5NXfKPYcqxdq3Yct/2qNnmWRZVMH7luFZqn eDH5p2h4TqROCiRChNrgvdohvrtM8zi28Ey5fRWLI31Xuno4xG7PjzMhlhY+lwzb2+ 1CSF1x4PjOzYtdRngZazwoqOJElurTS2yXIUQB0qvU/lEqG9aOFWjewZ1TQHRkuASS k+/31ZXA/T3/a6/UO+fJjk2vbdg3TEpkU6hbdAdURZH7J8uEDcMVJ65lEDT2XALCJd 1zjhYbO1TDQ24HqSfc67QKoMRtMKk2PxB7T6YL1vMVw1zVgI6AgS7R8ZRmk/7cBh0s layUMKAVVehpQ== Message-ID: <468db472-3298-0f3d-e000-29aed1abcd91@collabora.com> Date: Wed, 16 Mar 2022 22:36:14 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Cc: usama.anjum@collabora.com, Andrew Morton , Shuah Khan , kernel@collabora.com, Will Deacon , linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit Content-Language: en-US To: Gabriel Krisman Bertazi References: <20220315085014.1047291-1-usama.anjum@collabora.com> <20220315085014.1047291-2-usama.anjum@collabora.com> <871qz3ndji.fsf@collabora.com> From: Muhammad Usama Anjum In-Reply-To: <871qz3ndji.fsf@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 954D4180012 X-Stat-Signature: r4gi6ce4fjfprxa1iopid1bh7x8zufwy Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b="IiPUZ/OA"; spf=pass (imf16.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.227 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=none) header.from=collabora.com X-HE-Tag: 1647452183-533900 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: On 3/16/22 1:53 AM, Gabriel Krisman Bertazi wrote: > Muhammad Usama Anjum writes: > >> From: Gabriel Krisman Bertazi > > Hi Usama, > > Please, cc me on the whole thread. I didn't get the patch 1/2 or the > cover letter. > Sorry, I'll correct it. >> This introduces three tests: >> 1) Sanity check soft dirty basic semantics: allocate area, clean, dirty, >> check if the SD bit is flipped. >> 2) Check VMA reuse: validate the VM_SOFTDIRTY usage >> 3) Check soft-dirty on huge pages >> >> This was motivated by Will Deacon's fix commit 912efa17e512 ("mm: proc: >> Invalidate TLB after clearing soft-dirty page state"). I was tracking the >> same issue that he fixed, and this test would have caught it. >> >> CC: Will Deacon >> Signed-off-by: Gabriel Krisman Bertazi >> Signed-off-by: Muhammad Usama Anjum >> --- >> V3 of this patch is in Andrew's tree. Please drop that. > > v3 is still in linux-next and this note is quite hidden in the middle of > the commit message. I've tried to put this message at the top of the changelog. I can add "Note" in the start of it. What can be some other way to highlight this kind of important message? >> >> Changes in V4: >> Cosmetic changes >> Removed global variables >> Replaced ksft_print_msg with ksft_exit_fail_msg to exit the program at >> once >> Some other minor changes >> Correct the authorship of the patch >> >> Tests of soft dirty bit in this patch and in madv_populate.c are >> non-overlapping. madv_populate.c has only one soft-dirty bit test in the >> context of different advise (MADV_POPULATE_READ and >> MADV_POPULATE_WRITE). This new test adds more tests. >> >> Tab width of 8 has been used to align the macros. This alignment may look >> odd in shell or email. But it looks alright in editors. > > I'm curious if you tested reverting 912efa17e512. Did the new versions > of this patch still catch the original issue? Yeah, it did after I reverted the patch and fixed build errors because of some function's signature change and one test failed and hence issue is caught: TAP version 13 1..5 # dirty bit was 0, but should be 1 (i=1) not ok 1 Test test_simple ok 2 Test test_vma_reuse reused memory location ok 3 Test test_vma_reuse dirty bit of previous page ok 4 # SKIP Test test_hugepage huge page allocation ok 5 # SKIP Test test_hugepage huge page dirty bit # Totals: pass:2 fail:1 xfail:0 xpass:0 skip:2 error:0 >> Test output: >> TAP version 13 >> 1..5 >> ok 1 Test test_simple >> ok 2 Test test_vma_reuse reused memory location >> ok 3 Test test_vma_reuse dirty bit of previous page >> ok 4 Test test_hugepage huge page allocation >> ok 5 Test test_hugepage huge page dirty bit >> # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0 >> >> Or >> >> TAP version 13 >> 1..5 >> ok 1 Test test_simple >> ok 2 Test test_vma_reuse reused memory location >> ok 3 Test test_vma_reuse dirty bit of previous page >> ok 4 # SKIP Test test_hugepage huge page allocation >> ok 5 # SKIP Test test_hugepage huge page dirty bit >> # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0 [..] >> + >> +#define PAGEMAP "/proc/self/pagemap" >> +#define CLEAR_REFS "/proc/self/clear_refs" >> +#define MAX_LINE_LENGTH 512 > > MAX_LINE_LENGTH is no longer used after check_for_pattern was dropped. > > Can't the previous defines and file handling functions also go the > vm_util.h? > I don't want to make changes in other two tests. I just want to move some functions which we need for this test into vm_util.h while keeping changes less. >> +#define TEST_ITERATIONS 10000 >> + >> +static void test_simple(int pagemap_fd, int pagesize) >> +{ >> + int i; >> + char *map; >> + >> + map = aligned_alloc(pagesize, pagesize); >> + if (!map) >> + ksft_exit_fail_msg("mmap failed\n"); >> + >> + clear_softdirty(); >> + >> + for (i = 0 ; i < TEST_ITERATIONS; i++) { >> + if (pagemap_is_softdirty(pagemap_fd, map) == 1) { >> + ksft_print_msg("dirty bit was 1, but should be 0 (i=%d)\n", i); >> + break; >> + } >> + >> + clear_softdirty(); >> + map[0]++; > > > This will overflow several times during TEST_ITERATIONS. While it is > not broken, since we care about causing the page fault, it is not > obvious. Can you add a comment or do something like this instead? > > map[0] = !map[0]; Yeah, it is less obvious. I'll add a comment -- Muhammad Usama Anjum