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 C8AE5D3DEAB for ; Fri, 18 Oct 2024 18:37:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5A2056B00AC; Fri, 18 Oct 2024 14:37:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 552986B00AD; Fri, 18 Oct 2024 14:37:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 41A156B00AE; Fri, 18 Oct 2024 14:37:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 1D4FD6B00AC for ; Fri, 18 Oct 2024 14:37:31 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 112E1A1B57 for ; Fri, 18 Oct 2024 18:37:08 +0000 (UTC) X-FDA: 82687580682.26.355E7FF Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf16.hostedemail.com (Postfix) with ESMTP id A8410180018 for ; Fri, 18 Oct 2024 18:37:17 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=KmF0Fi3m; spf=pass (imf16.hostedemail.com: domain of broonie@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729276501; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=t2V0wjCsz/fZLJ9kIpYF3zt4nbMqDq1TE5uhw0XGxpQ=; b=OOXFKoIv8HUP6PEqp0RXnVLzvOZjOAzesTcJsZ3Z+HhgL9eEI4d1FRnCIZjE09in3tHh7i rT534HTHHfCtyYOxj1dlL51ocJRnu8ZSsv2GlGWGD29o/N3ZLLghRLTU8vMJ45bgdkiz2d eA8GymJwjnHi4pobycnsiOXA0WPMcbo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729276501; a=rsa-sha256; cv=none; b=ltLbUNGbJe/uwZ8t9KSc2NAgKSI1hReZz00zCiLFHxbQJUXdb24ZdgmeR30wxodvM1YokR 3DjOxRuVJGvodU7X/E1j9mJFrmMBJTunJ5hLPhoDPfhLGr3ntpDAc184EHzxMyT5Ig0fBI QwPu2sg1V1QGAXzlYTVTeWqrBME9K4Y= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=KmF0Fi3m; spf=pass (imf16.hostedemail.com: domain of broonie@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=broonie@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 521CAA448D8; Fri, 18 Oct 2024 18:37:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 42D6FC4CEC3; Fri, 18 Oct 2024 18:37:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1729276648; bh=dSqi9IfAa5l3jYcAxFqh5y6voNSnsY/4yK4Ky+osyzY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KmF0Fi3mLpV6GjbQaCRA6EA3/hmTHOuYNb0OlV7FM0aKT1stkxnb+6quUky7+IjhV L6o+NKUWoPLD0kfZrKO5Nb0eGG8/hecQqI500alUD6JS5TbiEX+cUIVFI690ETpBfr aSuR3sl03MMg5zFMW1AVohkqFFSrBrTDvdxUq+P5TWeIaX4iXhjZI0rWojBZSlTWN0 D3tPgPmiXLxe7D8jCX6zsKKNtZg4G4AB6S+35NwFNVl4CNDHes91AISvxq1JAoQibD QBJl6sB+AlhRRTSEt3HTSH9Ldg2wQPdCNiEi91k5J1rNQEX29k3ixX7+dbWr9Ty8Du ijJKjWIrDr72A== Date: Fri, 18 Oct 2024 19:37:22 +0100 From: Mark Brown To: Jeff Xu Cc: Muhammad Usama Anjum , Lorenzo Stoakes , akpm@linux-foundation.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, pedro.falcato@gmail.com, willy@infradead.org, vbabka@suse.cz, Liam.Howlett@oracle.com, rientjes@google.com, keescook@chromium.org Subject: Re: [PATCH v3 4/5] selftests/mseal: add more tests for mmap Message-ID: References: <4944ce41-9fe1-4e22-8967-f6bd7eafae3f@lucifer.local> <1f8eff74-005b-4fa9-9446-47f4cdbf3e8d@sirena.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="P9lxhlovMTxuF4jF" Content-Disposition: inline In-Reply-To: X-Cookie: What is the sound of one hand clapping? X-Stat-Signature: a71oefnwd17tbmm716n1zaaki1w7om9d X-Rspamd-Queue-Id: A8410180018 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1729276637-466144 X-HE-Meta: U2FsdGVkX1+iCTybvikmEWJJo0Z36d655j5E3y+/rH0Xu3v8uLHo9QxLfM03gvGmCZcnZMY7Sfpfdv6Wwbog5CyrRZ41cnsj2D0ujyMkN9WohaarSjqDvgMkylIDNunftMeD64snd932Tk9x2F57lynPXvdEZUlLjOXg4e3Ag68XooqXQjLef//oKRsM1cEGu25yO7ej0lAWOXvTOb0cEssApMLsoNHykwx+XsY33aXFSXQCKaXsxI+z2G0UpCe3PzzzdI121mJttje6eflSU2XFumq6suDR1N1EW7+tq4xTVKVW0+MzyNt2s2LrDPJXL/TO6MUgiiqahFdVmkfE1IQPvX2AeGTMZAwOZfNeXE/otASdp6wwHl7b8baIj8RLXTVPVr1mnFZ8w05SUwoM8A0Tze4DHawtBazs64fvzILv9raawA2HXyES/tKeJjG/Dbv2NDMt4YhOFCFdl33NyIs1OpgbcJtK2ovSwasQ0gv0TUpE1ryR60FjT7UPHN2kiq0zu5rAlV7QMZoU6iM5xoM12o/HosxABzEr0EQjqUHLEQcvysctTUqFiN77GhZ5sGYsqommNcjIDUmQz/wuc59Gz3VrLkAWJD/JFTj5Rs0CBq//NDpcQ4ssrCMTA2Ca5i/M7pSmQFfaIxF4+abEbjhz7exys8fgnt2GiPLWHxWf2EZPu5nMsj5OxyyAuf66/2tQeZ5KcleatPjMaiimlfOFC3+hDKjfBlIS41ewpApRox8FM5/NDL44/8wBfRG0+FXizvH/4cQfveTw1m7AO0fcSK4o3rUjFcJpiBJCdK+6u9P99SwlSC2JFClhut1ONlcHCFz2vnSrfY0qfhWxv/sKf7DTvoVLrObh5n7vVQaOGxE69PIE4lPACXeMkh0ybqwiP9kK3LRYE4CLePQBalcgpX5gCvoB5AGEM6J53Nn78WVYewgMTLk8MClDngCyD0leOWoyMjLJWXe8Itg 7vrk0UE7 DtX70l1DjS8Akm9kl3gx7IzpQJD82UyukD9PdtCPHIl+RfoDVcKEOfuTweB+Ajs6FiPBM8aXDNyX+A7e+9oDGYTsQtmoPREabfkImqjLGIhtpBVaxsPWa4/0JLSFWhGkQHlGhRiYtB7G/hN54HVyDlEqmzRx+dMN1pmDpCmeurz38ne2H0dKsc1Hriuqirbs8b13yJLuu+Rb8OH5qzYiQL2n/YplcwdA6q3b06yCTCk3zzS0+SAw0/s9DxqD/5dEQxueu18Gp/nmvtBlYSLRA31ET2jWSE2SLvIzFFvh3IBOL2NG+l+KECpAAtuoZnNGke4jxMc1R9q1Qg728hzmKQgNHlHlRe0ORHt46hkGs9mClFIG+a2/CfVroKKn3ff20PT8MzeTAg5ohbxP79RBZei2NtO64h0xiJDn+ 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: --P9lxhlovMTxuF4jF Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 18, 2024 at 11:06:20AM -0700, Jeff Xu wrote: > On Fri, Oct 18, 2024 at 6:04=E2=80=AFAM Mark Brown w= rote: > > The problem is that the macro name is confusing and not terribly > > consistent with how the rest of the selftests work. The standard > > kselftest result reporting is > > ksft_test_result(bool result, char *name_format, ...); > > > > so the result of the test is a boolean flag which is passed in. This > > macro on the other hand sounds like a double negative so you have to > > stop and think what the logic is, and it's not seen anywhere else so > > nobody is going to be familiar with it. The main thing this is doing is > > burying a return statement in there, that's a bit surprising too. > Thanks for explaining the problem, naming is hard. Do you have a > suggestion on a better naming? Honestly I'd probably deal with this by refactoring such that the macro isn't needed and the tests follow a pattern more like: if (ret !=3D 0) { ksft_print_msg("$ACTION failed with %d\n", ret); return false; } when they encouter a failure, the pattern I sketched in my earlier message, or switch to kselftest_harness.h (like I say I don't know if the fork()ing is an issue for these tests). If I had to have a macro it'd probably be something like mseal_assert(). > > I'll also note that these macros are resulting in broken kselftest > > output, the name for a test has to be stable for automated systems to be > > able to associate test results between runs but these print =2E... > > which includes the line number of the test in the name which is an > > obvious problem, automated systems won't be able to tell that any two > > failures are related to each other never mind the passing test. We > > should report why things failed but it's better to do that with a > > ksft_print_msg(), ideally one that's directly readable rather than > > requiring someone to go into the source code and look it up. > I don't know what the issue you described is ? Are you saying that we > are missing line numbers ? it is not. here is the sample of output: No, I'm saying that having the line numbers is a problem. > Failure in the second test case from last: > ok 105 test_munmap_free_multiple_ranges > not ok 106 test_munmap_free_multiple_ranges_with_split: line:2573 > ok 107 test_munmap_free_multiple_ranges_with_split Test 106 here is called "test_munmap_free_multiple_ranges_with_split: line:2573" which automated systems aren't going to be able to associate with the passing "test_munmap_free_multiple_ranges_with_split", nor with any failures that occur on any other lines in the function. > I would image the needs of something similar to FAIL_TEST_IF_FALSE is > common in selftest writing: > 1> lightweight enough so dev can pick up quickly and adapt to existing > tests, instead of rewriting everything from scratch. > 2> assert like syntax > 3> fail the current test case, but continue running the next test case > 4> take care of reporting test failures. Honestly this just sounds and looks like kselftest_harness.h, it's ASSERT_ and EXPECT_ macros sound exactly like what you're looking for for asserts. The main gotchas with it are that it's not particularly elegant for test cases which need to enumerate system features (which I don't think is the case for mseal()?) and that it runs each test case in a fork()ed child which can be inconvenient for some tests. The use of fork() is because that make the overall test program much more robust against breakage in individual tests and allows things like per test timeouts. --P9lxhlovMTxuF4jF Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmcSquEACgkQJNaLcl1U h9Co7Qf+IFX02DfWKnGFQmrXx/BU2tcsHpr6ALzD+Pf5eVjuzGXP2trUWphjlykz xQ1iS/lr0f+4xBGnAVbjryUNyIyPqPJ1an4APNVoEBIb0H4PY6i5vQglTj4qxvVL 8RUQx3bMWMrfok1duAFTMm/Kpl94uPCH5TYTdvdM2iwrQnOqCsKDnSUItG+n9jVh GnArZl82biCNbQfGUdB01rMmMohltm7xfOiu2ZGzT3H+yPf3WXsP0epX12oLOP6j 13gOBbpJmARrQRdV14gUNl9WoRjblWTYGsjdVEL/m1eV+NC6Zq8QeSfs8MAiEU2v dvqGHf0ov9NhgPPLYl3TcjEhGRl7xw== =O77y -----END PGP SIGNATURE----- --P9lxhlovMTxuF4jF--