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 7895ECDB482 for ; Thu, 19 Oct 2023 15:43:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0DF5780095; Thu, 19 Oct 2023 11:43:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 08F688000B; Thu, 19 Oct 2023 11:43:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E71EE80095; Thu, 19 Oct 2023 11:43:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D27658000B for ; Thu, 19 Oct 2023 11:43:23 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 9389780515 for ; Thu, 19 Oct 2023 15:43:23 +0000 (UTC) X-FDA: 81362630286.19.A70A471 Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf16.hostedemail.com (Postfix) with ESMTP id C71F018001E for ; Thu, 19 Oct 2023 15:43:21 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=hhn8WFbs; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf16.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697730201; 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=0DPDsfu8aU8LPZp5ceCevOLz2QIEW4bMfW3ekc28nuA=; b=fv+hyzb8p0ZTWm+XdjR6IVulQ8cuNZ04quFFgnIV6I54s1xR4OgkNFGk+D/TO4wdh+29RC T0iXHsKfBl3r7DL4AX9ccet7InPkPZlJrenNh2MayMOZpobHFaIsJwNbBq4rnIvADRaFhx +ZW4fhfcWHIDNOaePKIFYe1ri9I+HP8= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=hhn8WFbs; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf16.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697730201; a=rsa-sha256; cv=none; b=JVqxLqB053YYjxwRCxth/MufITYK4ugJSUMR7nwtoQWkknXHDbQK6oV92QAj0E4xX9Qh2+ DxP1wVOo+4TsaQV5BdyQTYQr/ZsS8tnF30F0j1QOI/dtT6EmWiN7XMVjK0g6Zf611RkHeg ia7f+FVTgQECdhIVzkpYywsUJgbuqYk= Received: by mail-yb1-f169.google.com with SMTP id 3f1490d57ef6-d9a518d66a1so9358600276.0 for ; Thu, 19 Oct 2023 08:43:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697730201; x=1698335001; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=0DPDsfu8aU8LPZp5ceCevOLz2QIEW4bMfW3ekc28nuA=; b=hhn8WFbs4wNmOUnHtzcX90BG7s3cRoss15d3kPEqcemWXBTE1rY13TYVODoYkc61EO KQoCH+FCGaq8+W5asGccdRjT2RO34WYGDxaEwmr7i0q7TjNs5K6XkmiB2t8tJkU9kHgP BQ0GrdoD9j/8rRwW5qKPuqRoU+YBWZZkLqQxHBcJT/G+03DhiIFMSoVMT4omznmPawGb OJZKgDVFLDKcYEDCJqkRoKqlU7TF2SWKJfDPOEMZMzH86jE2RQT2vlfoJJiTJqp9HiXK YrL5roTuiQNdjrz85firISacTlEvzaLc9ouC7UFEE1qBlhLWDkJrRH74F4/5DaLVw1Bu lvlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697730201; x=1698335001; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0DPDsfu8aU8LPZp5ceCevOLz2QIEW4bMfW3ekc28nuA=; b=N3+81nm5BoRhIBPk/I+3NCPc401ivb+9H0QTHe/JwJKeBtCej+lvPPCRXAzmpTjGSZ USSCUocotpm6aJlDG6KOD/ax1q0WMFHtFZq4rv5B/0ZRACzJ5koeoNVGhUG+ddSlMH/q iTZDuasKfd28OziDGxH/afsYj1UWgNJrPNUJHLLQpQbvB1XNh3IMsjoWt+lw9rO65w0H oKb8EOhhYTrKWQNTXO0T5P/yhF+3tlegzNN6ef7k30odK1U+GKBRqMkbt4iy26OZu+fR iAOrjtrf1YZ4sWkcoDMznYETC8EZCebbPEOcUKEgaOkUx7iZzvGLT7FTwJfmcMFWOK7/ x1Uw== X-Gm-Message-State: AOJu0YxSZ86xf4flE3IcxMjTD7s5UaUz+d/tWH8tJQCv51TmXWxhhtZL Q/ExFJZAvX5zC0vVGykpIbZ+V/+zIHVq1fz+ZT0oPg== X-Google-Smtp-Source: AGHT+IHe28QnWKSdMzGbZBpSd2uD/FXtVN47Cyko18LnBywC9KLnDTZu5KY3QtF/uku8HpgQF5dunHQMdDre0XAaHhs= X-Received: by 2002:a05:6902:302:b0:d9b:6c9d:e6a with SMTP id b2-20020a056902030200b00d9b6c9d0e6amr2644471ybs.0.1697730200564; Thu, 19 Oct 2023 08:43:20 -0700 (PDT) MIME-Version: 1.0 References: <20231009064230.2952396-1-surenb@google.com> <20231009064230.2952396-4-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Thu, 19 Oct 2023 08:43:07 -0700 Message-ID: Subject: Re: [PATCH v3 3/3] selftests/mm: add UFFDIO_MOVE ioctl test To: Peter Xu Cc: akpm@linux-foundation.org, viro@zeniv.linux.org.uk, brauner@kernel.org, shuah@kernel.org, aarcange@redhat.com, lokeshgidra@google.com, david@redhat.com, hughd@google.com, mhocko@suse.com, axelrasmussen@google.com, rppt@kernel.org, willy@infradead.org, Liam.Howlett@oracle.com, jannh@google.com, zhangpeng362@huawei.com, bgeffon@google.com, kaleshsingh@google.com, ngeoffray@google.com, jdduke@google.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: C71F018001E X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: y1phthw5xrxqgafoaabbznceswt68xyp X-HE-Tag: 1697730201-378530 X-HE-Meta: U2FsdGVkX18AnkYu5hhC8I/EoUmSGGQjcMQl66SFhMpYRiU3y8VTDgLWywEklAuBMjfBcleX8I8z/aNVa7h4m5i1xv1pHhuB7QqawN9W9Q6UkFwERwL/i3nrDsb762UYWq+SBjL7mfwQSoSjvBfHsLTctEPBnOPv93jvgTIIZRmm6A8UlFW38YgZV+Y7+5NJamZhJGecli7hzetrolXiA7vBecE9e4t1FQEsXIICMZZk6AqanqhvP2wYZMljSWnWD2qKQg2IZ5OqNGWpVNZCd+c+O83uq5jJW3/AXHVn9anMNNkyVDhnDFGusxvubEPcfr3dl1PHcUkCbGgD2uGBAnRzaMC8RSZ98M2oQhTPVLpQWRzrRri9wKDY8HsBeh+AC528Sm5GGvG+dTmre7hq8QsNmPOf5/uiC2B1FvhyjgmsfT70XgqgiCD2jPLOHtNMq92uWvhY1U+JmlXyWlrAbXkKBIZ5LvnEF9DcLHZ3G4Zv/2OpmA0t2jSivX0raumw5E4Er1zV2MwWMIjQfB27+e+ar0DY43a3onrVLX5KGnx7jtoWVRFSozNopTDDv/iTqfH1e8bJqBHpbBMgAAo+Nv6EWcpPBUTDxKVo1c546oW9kStqg7f6VbzqoMcoc59IuVE+EIme78aXlUQZ/U1xnzBIULdGRVSxHYWe1p/1eai5SZ4etBIu6f1XWbXlNh0MKHDiOwE7A5PkCucPJwlmHNvzNCsxHcdYDtjC0nCNpYt2JtYv7njYJPqA43cU2SgTOLuWGqs8gxEn+7nCf4j4mU5IffG8PadfDgqCnnRtL9fl/kKgphFNaZP13dVBu2C5KxLLGuYE8pQ9SXlPGW31aPeOvFPwX2jB/8vAgdCy7EoaEugejAbsf0IOkwaZcbAYXVhHxj7Ej7Jgso2AIHyDCaa6ePKfkON+uaqeHYBl3O+ZOAM560vUMHHvMi8AGWFkImNCf7343nJuxf4dm4+ X80likSJ KZfr0t2KEN2DdIg1hy3KKnnIzmqbXI300b+B3rUMoNd/+RHEo3UWV4RI5inxkrxFUs/hhd2reeYkDp8ZXaMh+sXhM3BskQX2KZvwuQKKiQtkYRN+iSrZwFNZGt8mx29Cc+tR4tMFOBGuVDdd3PGU1pY/Qr3BLhldLMvLyGkZ4uTxBdEYtnQHKNprTMfNCExNxwF0hdeEsVui7S5fsEJ+J4T4Owtzm0Rocz9DWzPuD2nGhfJ062KnUmNu6gwDcWn7D5SHQqYHTFUVjSoqd4RRHDd7vPQ/stp7WMFNsB+Z7owEJCRuWlfLN97Q5Cg1tNpk8IVt5n9rRZqTxGmor0Xb631CeFWE73ukiaE52wDgbegcftljJrlPlwrjvUQ== 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 Thu, Oct 12, 2023 at 3:29=E2=80=AFPM Peter Xu wrote: > > On Sun, Oct 08, 2023 at 11:42:28PM -0700, Suren Baghdasaryan wrote: > > Add a test for new UFFDIO_MOVE ioctl which uses uffd to move source > > into destination buffer while checking the contents of both after > > remapping. After the operation the content of the destination buffer > > should match the original source buffer's content while the source > > buffer should be zeroed. > > > > Signed-off-by: Suren Baghdasaryan > > --- > > tools/testing/selftests/mm/uffd-common.c | 41 ++++++++++++- > > tools/testing/selftests/mm/uffd-common.h | 1 + > > tools/testing/selftests/mm/uffd-unit-tests.c | 62 ++++++++++++++++++++ > > 3 files changed, 102 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/s= elftests/mm/uffd-common.c > > index 02b89860e193..ecc1244f1c2b 100644 > > --- a/tools/testing/selftests/mm/uffd-common.c > > +++ b/tools/testing/selftests/mm/uffd-common.c > > @@ -52,6 +52,13 @@ static int anon_allocate_area(void **alloc_area, boo= l is_src) > > *alloc_area =3D NULL; > > return -errno; > > } > > + > > + /* Prevent source pages from collapsing into THPs */ > > + if (madvise(*alloc_area, nr_pages * page_size, MADV_NOHUGEPAGE)) = { > > + *alloc_area =3D NULL; > > + return -errno; > > + } > > Can we move this to test specific code? Ack. I think that's doable. > > > + > > return 0; > > } > > > > @@ -484,8 +491,14 @@ void uffd_handle_page_fault(struct uffd_msg *msg, = struct uffd_args *args) > > offset =3D (char *)(unsigned long)msg->arg.pagefault.addr= ess - area_dst; > > offset &=3D ~(page_size-1); > > > > - if (copy_page(uffd, offset, args->apply_wp)) > > - args->missing_faults++; > > + /* UFFD_MOVE is supported for anon non-shared mappings. *= / > > + if (uffd_test_ops =3D=3D &anon_uffd_test_ops && !map_shar= ed) { > > IIUC this means move_page() will start to run on many other tests... as > long as anonymous & private. Probably not wanted, because not all tests > may need this MOVE test, and it also means UFFDIO_COPY is never tested on > anonymous.. > > You can overwrite uffd_args.handle_fault(). Axel just added a hook which > seems also usable here. See 99aa77215ad02. Yes, I was thinking about adding a completely new set of tests for UFFDIO_MOVE but was not sure. With your confirmation I'll follow that path so that UFFDIO_COPY tests stay the same. > > > + if (move_page(uffd, offset)) > > + args->missing_faults++; > > + } else { > > + if (copy_page(uffd, offset, args->apply_wp)) > > + args->missing_faults++; > > + } > > } > > } > > > > @@ -620,6 +633,30 @@ int copy_page(int ufd, unsigned long offset, bool = wp) > > return __copy_page(ufd, offset, false, wp); > > } > > > > +int move_page(int ufd, unsigned long offset) > > +{ > > + struct uffdio_move uffdio_move; > > + > > + if (offset >=3D nr_pages * page_size) > > + err("unexpected offset %lu\n", offset); > > + uffdio_move.dst =3D (unsigned long) area_dst + offset; > > + uffdio_move.src =3D (unsigned long) area_src + offset; > > + uffdio_move.len =3D page_size; > > + uffdio_move.mode =3D UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES; > > + uffdio_move.move =3D 0; > > + if (ioctl(ufd, UFFDIO_MOVE, &uffdio_move)) { > > + /* real retval in uffdio_move.move */ > > + if (uffdio_move.move !=3D -EEXIST) > > + err("UFFDIO_MOVE error: %"PRId64, > > + (int64_t)uffdio_move.move); > > + wake_range(ufd, uffdio_move.dst, page_size); > > + } else if (uffdio_move.move !=3D page_size) { > > + err("UFFDIO_MOVE error: %"PRId64, (int64_t)uffdio_move.mo= ve); > > + } else > > + return 1; > > + return 0; > > +} > > + > > int uffd_open_dev(unsigned int flags) > > { > > int fd, uffd; > > diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/s= elftests/mm/uffd-common.h > > index 7c4fa964c3b0..f4d79e169a3d 100644 > > --- a/tools/testing/selftests/mm/uffd-common.h > > +++ b/tools/testing/selftests/mm/uffd-common.h > > @@ -111,6 +111,7 @@ void wp_range(int ufd, __u64 start, __u64 len, bool= wp); > > void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_args *ar= gs); > > int __copy_page(int ufd, unsigned long offset, bool retry, bool wp); > > int copy_page(int ufd, unsigned long offset, bool wp); > > +int move_page(int ufd, unsigned long offset); > > void *uffd_poll_thread(void *arg); > > > > int uffd_open_dev(unsigned int flags); > > diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testi= ng/selftests/mm/uffd-unit-tests.c > > index 2709a34a39c5..f0ded3b34367 100644 > > --- a/tools/testing/selftests/mm/uffd-unit-tests.c > > +++ b/tools/testing/selftests/mm/uffd-unit-tests.c > > @@ -824,6 +824,10 @@ static void uffd_events_test_common(bool wp) > > char c; > > struct uffd_args args =3D { 0 }; > > > > + /* Prevent source pages from being mapped more than once */ > > + if (madvise(area_src, nr_pages * page_size, MADV_DONTFORK)) > > + err("madvise(MADV_DONTFORK) failed"); > > Modifying events test is weird.. I assume you don't need this anymore aft= er > you switch to the handle_fault() hook. I think so but let me try first and I'll get back on that. > > > + > > fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK); > > if (uffd_register(uffd, area_dst, nr_pages * page_size, > > true, wp, false)) > > @@ -1062,6 +1066,58 @@ static void uffd_poison_test(uffd_test_args_t *t= args) > > uffd_test_pass(); > > } > > > > +static void uffd_move_test(uffd_test_args_t *targs) > > +{ > > + unsigned long nr; > > + pthread_t uffd_mon; > > + char c; > > + unsigned long long count; > > + struct uffd_args args =3D { 0 }; > > + > > + if (uffd_register(uffd, area_dst, nr_pages * page_size, > > + true, false, false)) > > + err("register failure"); > > + > > + if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args)) > > + err("uffd_poll_thread create"); > > + > > + /* > > + * Read each of the pages back using the UFFD-registered mapping.= We > > + * expect that the first time we touch a page, it will result in = a missing > > + * fault. uffd_poll_thread will resolve the fault by remapping so= urce > > + * page to destination. > > + */ > > + for (nr =3D 0; nr < nr_pages; nr++) { > > + /* Check area_src content */ > > + count =3D *area_count(area_src, nr); > > + if (count !=3D count_verify[nr]) > > + err("nr %lu source memory invalid %llu %llu\n", > > + nr, count, count_verify[nr]); > > + > > + /* Faulting into area_dst should remap the page */ > > + count =3D *area_count(area_dst, nr); > > + if (count !=3D count_verify[nr]) > > + err("nr %lu memory corruption %llu %llu\n", > > + nr, count, count_verify[nr]); > > + > > + /* Re-check area_src content which should be empty */ > > + count =3D *area_count(area_src, nr); > > + if (count !=3D 0) > > + err("nr %lu move failed %llu %llu\n", > > + nr, count, count_verify[nr]); > > All of above should see zeros, right? Because I don't think anyone boost= ed > the counter at all.. > > Maybe set some non-zero values to it? Then the re-check can make more > sense. I thought uffd_test_ctx_init() is initializing area_count(area_src, nr), so the source pages should contain non-zero data before the move. Am I missing something? > > If you want, I think we can also make uffd-stress.c test to cover MOVE to= o, > basically replacing all UFFDIO_COPY when e.g. user specified from cmdline= . > Optional, and may need some touch ups here and there, though. That's a good idea. I'll add that in the next version. Thanks, Suren. > > Thanks, > > > + } > > + > > + if (write(pipefd[1], &c, sizeof(c)) !=3D sizeof(c)) > > + err("pipe write"); > > + if (pthread_join(uffd_mon, NULL)) > > + err("join() failed"); > > + > > + if (args.missing_faults !=3D nr_pages || args.minor_faults !=3D 0= ) > > + uffd_test_fail("stats check error"); > > + else > > + uffd_test_pass(); > > +} > > + > > /* > > * Test the returned uffdio_register.ioctls with different register mo= des. > > * Note that _UFFDIO_ZEROPAGE is tested separately in the zeropage tes= t. > > @@ -1139,6 +1195,12 @@ uffd_test_case_t uffd_tests[] =3D { > > .mem_targets =3D MEM_ALL, > > .uffd_feature_required =3D 0, > > }, > > + { > > + .name =3D "move", > > + .uffd_fn =3D uffd_move_test, > > + .mem_targets =3D MEM_ANON, > > + .uffd_feature_required =3D UFFD_FEATURE_MOVE, > > + }, > > { > > .name =3D "wp-fork", > > .uffd_fn =3D uffd_wp_fork_test, > > -- > > 2.42.0.609.gbb76f46606-goog > > > > -- > Peter Xu > > -- > To unsubscribe from this group and stop receiving emails from it, send an= email to kernel-team+unsubscribe@android.com. >