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 7F486C76188 for ; Mon, 3 Apr 2023 20:24:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EAA6C900002; Mon, 3 Apr 2023 16:24:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E594B6B008C; Mon, 3 Apr 2023 16:24:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D480F900002; Mon, 3 Apr 2023 16:24:19 -0400 (EDT) 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 C5FFE6B008A for ; Mon, 3 Apr 2023 16:24:19 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 9F9FA80B9E for ; Mon, 3 Apr 2023 20:24:19 +0000 (UTC) X-FDA: 80641207038.25.7665BA2 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf17.hostedemail.com (Postfix) with ESMTP id F0B434001C for ; Mon, 3 Apr 2023 20:24:16 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf17.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680553457; 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; bh=12z27TE6VQBbAogAxz6FeUpqEHMEvWSC888OB+GlE4o=; b=vDD9okmDzAwbBAt8licBfzruSVvHZ9MEUIAm9d/BkQfiQb1fbkw73bH/O61S3nPTU7XU3D iD9Ng9bgyYXxeTpBOra3VSDxUWEyzFPsRqJggUt2VNzVschi9hK60mFObNuPh+ttKOcWC4 eq68sYqAUny/KAJFxCl/3V+B+zvuZGE= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf17.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680553457; a=rsa-sha256; cv=none; b=XGoGuH90b1fJrfGGdcHM1pzg7mgr0MEgACIQMwJYpSfyCDQcRF+9XTj7YpfamNDV/Ki5Sr 4l2+Vaq/Dzx83J3jNfnk9usTbLgcDVi/D+XvAljK95BwUN7430DxPlz1zMynYVn/ffMUna 2t3EYy06kJY7dtn9I+LbuY9MATify7c= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-640-m8WE5nrpPq6zHI6AceTdQQ-1; Mon, 03 Apr 2023 16:24:15 -0400 X-MC-Unique: m8WE5nrpPq6zHI6AceTdQQ-1 Received: by mail-qt1-f199.google.com with SMTP id v10-20020a05622a130a00b003e4ee70e001so14468416qtk.6 for ; Mon, 03 Apr 2023 13:24:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680553454; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=12z27TE6VQBbAogAxz6FeUpqEHMEvWSC888OB+GlE4o=; b=MCeUVd0DAvcfyCXDYRQ9TQhRKGUPzRdYARZVtLJqRwcPmYXOigZsnxLfCJK7VNgNrE BrX2Rhy5duj6gWpdzyUKPdqAMDiU73M1XMwwPNAKxddgqgTz7YDFLq5e4knWeFRlu+TT 5bChY20PM7WQPBnJdKu1gL7HI32Ozw2izawW1wQiYo6zBgnHlPEs103O3bZmWWXj56aN ypjdhIBFn7Bo4MEjSR1BuSE478vg3XhtIEEqq3+EfFZ/vMicVR7B8hUrz8oVEyJfVvAA u5RPAen1XkxcgCnnmicHPnnRHcxy4PHrIDZBSVWzeS9cyJwGsO5ry5xt1q3kX6w+1Ml9 TCVw== X-Gm-Message-State: AAQBX9dvUHfgUCSOmC6x4JPU+ne0y7tNw6S7a2y7rMDAFQeRj8gb8W4n uUdPxYt8PQ5hoYv3ZFQC4ocAXNvpkJCh/dea7wGucBlJBLkymKU2oX+5cuVWp7Ku+VWykgL8+nA DmxY3zupBgqs= X-Received: by 2002:a05:622a:1aa1:b0:3e3:7e6b:50ce with SMTP id s33-20020a05622a1aa100b003e37e6b50cemr28352855qtc.4.1680553454577; Mon, 03 Apr 2023 13:24:14 -0700 (PDT) X-Google-Smtp-Source: AKy350aJ5GZwPuDVCerdCcEtBCco7NVgNWOm1iTIkPAelFtiIg65vx1ypOOl8OxWAinXF8cHOqEMBg== X-Received: by 2002:a05:622a:1aa1:b0:3e3:7e6b:50ce with SMTP id s33-20020a05622a1aa100b003e37e6b50cemr28352811qtc.4.1680553454251; Mon, 03 Apr 2023 13:24:14 -0700 (PDT) Received: from x1n (bras-base-aurron9127w-grc-40-70-52-229-124.dsl.bell.ca. [70.52.229.124]) by smtp.gmail.com with ESMTPSA id r12-20020ac867cc000000b003e3910db4f1sm2766125qtp.35.2023.04.03.13.24.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Apr 2023 13:24:13 -0700 (PDT) Date: Mon, 3 Apr 2023 16:24:12 -0400 From: Peter Xu To: David Hildenbrand Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrea Arcangeli , Andrew Morton , Nadav Amit , Mike Kravetz , Axel Rasmussen , Leonardo Bras Soares Passos , Mike Rapoport Subject: Re: [PATCH 16/29] selftests/mm: UFFDIO_API test Message-ID: References: <20230330155707.3106228-1-peterx@redhat.com> <20230330160752.3107283-1-peterx@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Queue-Id: F0B434001C X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: g66qimri8k6rxwqbz4smjwwitkzz15ef X-HE-Tag: 1680553456-202332 X-HE-Meta: U2FsdGVkX19vfls077txjKrzDHF+nAbLKGe52p+2RVIFJ9YSBgsZycPsjX9FCoFBlDt0dWF0q+SBM1+UoP1ysXs1AcoUcbnhCqeaBkQvmY4UOnZNrHHTOv2VjTiHg2IT0GZlWPz5VaI0Mz8FXlVGMpjqp8ROm1kF2A0/G1TcQYkWUhaUIb4StJKWyoWngPvVKIhbrVF2dxQT4E34HC6A+wNQMKWNbdiGyxI7RXhiyWDb7jP0CWif81yIvzQ/8s+GCHEehRW4aMZkbTBmNlKds2bE8l1zBtKcAqzyp3IZmBUIU0p4nZMO9Lfeeq0t4yOpr21yeQtq91w2TS8ow8rCaS7UCze3mR6ksOg137RVWMMiWqE+V1cLu0fZfM8no9nPIX1y4Be+iE1Yrr9xpLPHK5NsSygiwcchLvatQwp0UO06OyPfzUAJpAod2Q5iKyTzjHsh5V3zAXj77zJmDAW/ZkPS1gFe1+UhdQJP5n69/wIXRgQR67nUkQhMDJ6TZlI9KuiKFpHZrX/ca6gVBKz9VzufMQCJCiBBpPdPW2i3s0xwR00BOtAG3e0JDHWKMiqmiAIasPyrdoD4oTStI3mMt0q0SsQbC6+RNC2kenb+4vLAZEENqpACRdEYU0doGFKjXFuROj7mdCM+wGBzpmWsIttNRX56yo1EUTvDcAaCo0kRNi9K1ICF3Zz6hhwb1VWr5RwX7JdphqsUjYdHhvwKY5DGX8sShMMxmiQVCiOG+OmVjSshiny9WHxxbvIVw4I5DrZHQJKvZAyXyK65VfOTBZQ0AVwBXzz6pgvbEFyxYYk8rpbjMc4tcWr5m2nv/paWwbgiCpqTH6Bi1c30Y0BfV/n7X65aWnv6Uf+LBvS1vC8ExPg2lWAD7tcSr2Zu/CgNICNSiiJWSikgH88UOxUjRPIl12i91fgVTyD6XX6nudckbOvDQ02nwuMve6cgbCBCAyieX4LK4wku+OFwkop 7RT8MlpJ UewXx9FafgTXZVMeN2lCvL/F+DVnzFGZVhP8PPHodBTK1Omn8/z84AvWOpOukJv+Z8l4l0rS6RcRsNOaPd8I+JFcgBG1FOdT9ikIeAkTbqlwjV18EoKN1RSBcriESKLGyqTWKDTbFs6Nh/+idzWcyl+uuYY1PI2qMJqAMR1VOgaEACbfCCB1u4EAS/ulImTk0GNFxFam41cikBsAlnaRgAWycL/kxYV0cqdT4Pwrle3AK4hMFzH7E74r3UpmVB5yYtFtrUbD+Ow/eGXSXZxDK6RhDT1mkRrqnzZYgGvPNBMVjpJYK+vdRis7gJt0jEeFehrBkpIFFgTh/6m6lQ0jOo7aFdTkBtYT77FvXoVMxh3XPVJ8czRIMxFcxP7FT+4bOqLlbfpcR44aOhSAtNmhqi/dDQbspEUPL6Rzq X-Bogosity: Ham, tests=bogofilter, spamicity=0.156648, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Apr 03, 2023 at 09:06:26PM +0200, David Hildenbrand wrote: > On 03.04.23 18:43, Peter Xu wrote: > > On Mon, Apr 03, 2023 at 09:59:50AM +0200, David Hildenbrand wrote: > > > There is ksft_print_msg, ksft_test_result, ksft_test_result_fail, ... do we > > > maybe want to convert properly to ksft while already at it? > > > > Yes, I started with trying to use that but found that there're not a lot of > > things that I can leverage. > > > > Starting with ksft_set_plan() - I think this is something we call first. I > > want the current unit test to skip everything if UFFD API test failed here, > > then I need to feed in a dynamic number of "plan" into ksft_set_plan(). > > But I never know after I ran the 1st test.. > > In cow.c I did that. Getting the number of tests right can be challenging > indeed. IMHO the major thing is not about not easy to set, it's about there's merely no benefit I can see of having that calculated at the start of a test. There's one thing it can do, that is when calling ksft_finished() it can be used to know whether all tests are run, but sadly here we're calculating everything just to make it match.. so it loses its last purpose.. IMHO. > > Basic "feature availability" checks would go first (is uffd even around?), > and depending on that you can set the plan. > > For everything else, you can skip instead of test, so it will still be > accounted towards the plan. > > > > > I can call ksft_set_plan() later than this, but it misses a few tests which > > also looks weird. > > Yeah, it would be nice to simply make ksft_set_plan() optional. For example, > make ksft_print_cnts() skip the comparison if ksft_plan == 0. At least > ksft_exit_skip() handles that already in a descend way (below). > > > > > It also seems to not really help anything at all and not obvious to use. > > E.g. ksft_finished() will reference ksft_plan then it'll trigger > > ksft_exit_fail() but here I want to make it SKIP if the 1st test failed > > simply because the kernel probably doesn't have CONFIG_USERFAULTFD. > > You'd simply do that availability check first and then use ksft_exit_skip() > in case not available I guess. > > > > > Another example: I never figured what does x{fail|pass|skip} meant in the > > header.. e.g. ksft_inc_xfail_cnt() is used nowhere so I cannot reference > > either. Then I don't know when I should increase them. > > In cow.c I have the following flow: > > ksft_print_header(); > ksft_set_plan(); > ... tests ... > err = ksft_get_fail_cnt(); > if (err) > ksft_exit_fail_msg(); > return ksft_exit_pass(); > > That gives me: > > # [INFO] detected THP size: 2048 KiB > # [INFO] detected hugetlb size: 2048 KiB > # [INFO] detected hugetlb size: 1048576 KiB > # [INFO] huge zeropage is enabled > TAP version 13 > 1..190 > ... > # Totals: pass:87 fail:0 xfail:0 xpass:0 skip:103 error:0 > > > I didn't use xfail or xpass so far, but what I understood is that these are > "expected failures" and "expected passes". fail/pass/skip are straight Yes, xfail can be expressed that way, but maybe not xpass? Otherwise it's hard to identify what's the difference between xpass and pass, because IIUC pass also means "expected to pass". > forward. > ksft_test_result_fail()/ksft_test_result_pass()/ksft_test_result_skip() are > used to set them. > > You'd do availability checks before ksft_set_plan() and fail with a > ksft_exit_skip() if the kernel doesn't support it. Then, you'd just use > ksft_test_result_fail()/ksft_test_result_pass()/ksft_test_result_skip(). > > > > > In short, to make the unit test behave as expected, I figured I'll just > > write these few helpers and that's good enough for this unit test. That > > takes perhaps 5 min anyway and isn't hugely bad for an unit test. > > > > Then I keep the exit code matching kselftests (KSFT_SKIP, etc.). > > > > What I can do here, though, is at least reuse the counters, e.g: > > > > ksft_inc_pass_cnt() / ksft_inc_fail_cnt() > > > > There's no ksft_inc_skip_cnt() so, maybe, I can just reuse > > ksft_inc_xskip_cnt() assuming that counts "skip"s? > > > > Let me know if you have better ideas, I'll be happy to switch in that case. > > I guess once you start manually increasing/decreasing the cnt, you might be > abusing the ksft framework indeed and are better off handling it differently > :D I'm serious considering that to address your comment here, to show that I'm trying my best to use whatever can help in this test case. :) Here reusing it would avoid a few bytes in the bss, which is still beneficial so I can do. At least I'm sure xskip is for skipping now, so I know what to use. PS: one other reason I didn't use the header is also because I prefer the current output of uffd-self-tests.c. I didn't say anything because I think it's stingy.. So let's keep this in a trivial small PS. After all, the print format is half of what the header provides functional-wise. I'll see what I can come up with at last in the next version, thanks David. -- Peter Xu