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 X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2B38C43460 for ; Thu, 15 Apr 2021 18:03:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7055C6137D for ; Thu, 15 Apr 2021 18:03:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7055C6137D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D7F346B0036; Thu, 15 Apr 2021 14:03:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D05356B006C; Thu, 15 Apr 2021 14:03:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B317A6B0070; Thu, 15 Apr 2021 14:03:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0194.hostedemail.com [216.40.44.194]) by kanga.kvack.org (Postfix) with ESMTP id 8D7186B0036 for ; Thu, 15 Apr 2021 14:03:53 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 411723638 for ; Thu, 15 Apr 2021 18:03:53 +0000 (UTC) X-FDA: 78035374746.10.8965B2E Received: from mail-il1-f181.google.com (mail-il1-f181.google.com [209.85.166.181]) by imf07.hostedemail.com (Postfix) with ESMTP id 73C6DA0000FD for ; Thu, 15 Apr 2021 18:03:52 +0000 (UTC) Received: by mail-il1-f181.google.com with SMTP id 7so19250831ilz.0 for ; Thu, 15 Apr 2021 11:03:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jMi3JQh4ceVfV/j7Lf7DWzkm4A/CG7sUwuBKTI9Y7vs=; b=LOWfqGRgHZ2GaLk4r0jnjtm6hmUQDMtLwyD23G/zSp+dciXK8p9hxW5dpWlieJWVov 9fZxBC9Gq0vChe5/SI2Xz2rZtf5evjuecN76OfNiPb1n7JvJtsnyuPbcU7lyolclN6h9 xm/USSXnuwH42Jp23W8TX1U8yNA3aiVNF0zBR+WhMg086kxGgjnkrWSXlZ2K1t+dKvX5 mz6u4JCxYhvmR84+sH+l+BSsbQc2DhNnetCk62Xcxyf/eFiJ8bkw8HN++VM57h/YDUKu p8uMAQs2++W6WuNhOtTfjydZwv7nX0X3/sjt3phwy8sk76atjR1aN3znqw1PgpCPveT8 oH7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jMi3JQh4ceVfV/j7Lf7DWzkm4A/CG7sUwuBKTI9Y7vs=; b=pxK3NEvuvyKvQWnxhpExxo+RZWH/leCfjeP6jmJxcNsREDCHJomZ+AdMNOij4HUwiG ZXpmazjoatW+RXL4ydy8WaryFO66DooFrhgrxGMUSt36kDt6AoJLCw1CMv0rFprbXTx3 /Wjl+M3KtxqTS7aPlKXAuDIo24I2Wqb4TS9eqy57A97/vn4zntZFoWbXdCdTHpG9SFT8 U9zjoS+wyaN2I+ca5TrVEWsrPKO9HCkYTIJBZK8hrU9CvQmwqKtV4zW3wQ74Zs1gUOXP sbSmf1p0KvLDdb0cptZb1E4vShlMOeDmJY8E8jxaI6lNy1h4odEFB0fJZDU1W5OiPXSP 7HbA== X-Gm-Message-State: AOAM532zQd0b8ZflDulpfgE7teGaXpa84R5S4hotb1ODU3Vas9RMDI/C pmXehQ7uZjDh278uwkjQboF/P0WR/jjQo9dpjjWWPQ== X-Google-Smtp-Source: ABdhPJzLg34mG/2F8echbVGsALC+cjlSLd1cDv+gDbhlWGfCHOBwcNyvJAecuz7DsvJdtaeQICx6VrotmYdZLAXz92k= X-Received: by 2002:a05:6e02:1d06:: with SMTP id i6mr1346852ila.165.1618509831881; Thu, 15 Apr 2021 11:03:51 -0700 (PDT) MIME-Version: 1.0 References: <20210413051721.2896915-1-axelrasmussen@google.com> <20210413051721.2896915-8-axelrasmussen@google.com> <20210413201535.GD4440@xz-x1> In-Reply-To: <20210413201535.GD4440@xz-x1> From: Axel Rasmussen Date: Thu, 15 Apr 2021 11:03:14 -0700 Message-ID: Subject: Re: [PATCH v2 7/9] userfaultfd/selftests: reinitialize test context in each test To: Peter Xu Cc: Alexander Viro , Andrea Arcangeli , Andrew Morton , Hugh Dickins , Jerome Glisse , Joe Perches , Lokesh Gidra , Mike Kravetz , Mike Rapoport , Shaohua Li , Shuah Khan , Stephen Rothwell , Wang Qing , linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, LKML , linux-kselftest@vger.kernel.org, Linux MM , Brian Geffon , "Dr . David Alan Gilbert" , Mina Almasry , Oliver Upton Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 73C6DA0000FD X-Stat-Signature: j9gdtuc676g719ktmyncssjjoaehr6ir X-Rspamd-Server: rspam02 Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf07; identity=mailfrom; envelope-from=""; helo=mail-il1-f181.google.com; client-ip=209.85.166.181 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618509832-867287 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 Tue, Apr 13, 2021 at 1:15 PM Peter Xu wrote: > > On Mon, Apr 12, 2021 at 10:17:19PM -0700, Axel Rasmussen wrote: > > Currently, the context (fds, mmap-ed areas, etc.) are global. Each test > > mutates this state in some way, in some cases really "clobbering it" > > (e.g., the events test mremap-ing area_dst over the top of area_src, or > > the minor faults tests overwriting the count_verify values in the test > > areas). We run the tests in a particular order, each test is careful to > > make the right assumptions about its starting state, etc. > > > > But, this is fragile. It's better for a test's success or failure to not > > depend on what some other prior test case did to the global state. > > > > To that end, clear and reinitialize the test context at the start of > > each test case, so whatever prior test cases did doesn't affect future > > tests. > > > > This is particularly relevant to this series because the events test's > > mremap of area_dst screws up assumptions the minor fault test was > > relying on. This wasn't a problem for hugetlb, as we don't mremap in > > that case. > > > > Signed-off-by: Axel Rasmussen > > --- > > tools/testing/selftests/vm/userfaultfd.c | 221 +++++++++++++---------- > > 1 file changed, 127 insertions(+), 94 deletions(-) > > > > diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c > > index 1f65c4ab7994..0ff01f437a39 100644 > > --- a/tools/testing/selftests/vm/userfaultfd.c > > +++ b/tools/testing/selftests/vm/userfaultfd.c > > @@ -89,7 +89,8 @@ static int shm_fd; > > static int huge_fd; > > static char *huge_fd_off0; > > static unsigned long long *count_verify; > > -static int uffd, uffd_flags, finished, *pipefd; > > +static int uffd = -1; > > +static int uffd_flags, finished, *pipefd; > > static char *area_src, *area_src_alias, *area_dst, *area_dst_alias; > > static char *zeropage; > > pthread_attr_t attr; > > @@ -342,6 +343,121 @@ static struct uffd_test_ops hugetlb_uffd_test_ops = { > > > > static struct uffd_test_ops *uffd_test_ops; > > > > +static int userfaultfd_open(uint64_t *features) > > +{ > > + struct uffdio_api uffdio_api; > > + > > + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK); > > Keep UFFD_USER_MODE_ONLY? > > [...] > > > @@ -961,10 +1045,9 @@ static int userfaultfd_zeropage_test(void) > > printf("testing UFFDIO_ZEROPAGE: "); > > fflush(stdout); > > > > - uffd_test_ops->release_pages(area_dst); > > - > > - if (userfaultfd_open(0)) > > + if (uffd_test_ctx_clear() || uffd_test_ctx_init(0)) > > return 1; > > Would it look even nicer to init() at the entry of each test, and clear() after > finish one test? I slightly prefer clearing at the beginning, as it means we don't need to depend on the previous test being correct for this test to function. And, we don't need more complex error handling in the test cases to make sure we don't mess things up for the next test. But, two things we can do to clean this up as-is: The initialization function can just call clear itself, so tests don't need to worry about it. And, with err(), we don't need these functions to return an int any more. I'll send a version like that, we can see how it looks. > > > + > > uffdio_register.range.start = (unsigned long) area_dst; > > uffdio_register.range.len = nr_pages * page_size; > > uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING; > > The rest looks good to me. Thanks, > > -- > Peter Xu >