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 1A315C3DA7A for ; Mon, 2 Jan 2023 13:58:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 93AA38E0002; Mon, 2 Jan 2023 08:58:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8E9D98E0001; Mon, 2 Jan 2023 08:58:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7B2778E0002; Mon, 2 Jan 2023 08:58:19 -0500 (EST) 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 6AB488E0001 for ; Mon, 2 Jan 2023 08:58:19 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 46452A08AF for ; Mon, 2 Jan 2023 13:58:19 +0000 (UTC) X-FDA: 80310013518.25.E76CADC Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by imf13.hostedemail.com (Postfix) with ESMTP id 8D93720007 for ; Mon, 2 Jan 2023 13:58:17 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=qm0uQwki; spf=pass (imf13.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1672667897; a=rsa-sha256; cv=none; b=wp42X+fFa3kd3Nw3noDMUEU9O7O/J3TQJv8XjGBIT2TfMmQOF8Bg5XmNy/kmsS4mfDj5WV KdQ1velPIePr/OsHLz0xztKPBihUaCQ4ZZaDeoZHW/2nFoWuvz+g8PuaiGs6bqLG6VpOoU lOr0LDtPtn5m3Vlqk8B1F+oUuSQJrRA= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=qm0uQwki; spf=pass (imf13.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1672667897; 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=nphmtWm1iGTaK7XYzssdMMXgZVHnsgqaAFS3kznMFEA=; b=1t7niVqWasvtduoEv9uG8YJILD5ETsfLcrTsiYItd7CBcw/6rnly8c1U9TwRVtS2Fan79V ZyWVFbL/ymYS9MFyrsbUqJETJuPIuitadlQ29CL1v3/9u8IHuJ+Aayn8eH9GQV7+dzGFHq eucXJs5ogkc8xvnJ8l8QzUT+H4g+WdY= Received: by mail-wr1-f53.google.com with SMTP id z10so26333309wrh.10 for ; Mon, 02 Jan 2023 05:58:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=nphmtWm1iGTaK7XYzssdMMXgZVHnsgqaAFS3kznMFEA=; b=qm0uQwkivEm3WjQSYHXx+e/k9LfmnHS3X0LeE9N8GAMvTjcUemkm/eNEtZFY7EK8Pe gMMqRqHG99iQ+gbKTqQjLBAn4uuiJJHp779KhANWDk7eVJZ7VL5TCJhYKf0jWhbnYuQT X/DNMCmJK+RsjJJxlKnrLaCrBfmk504y8/ZJwkIA3NZKX6tk+FKo9ByX1HYzPmoBbVz2 xq6fYsCymVN9wvkCWvHqsew7asFjsPFByFPjcEZpvZH2jaAgFJiPpFtVfHYJyRJjKG6e A0IRb7oWeu5DBXMYvyJzahrU8gw1ua9pYzp/0EiJJORUqcp1Ihq94AHw+e/IlfaEg5Zt b4aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=nphmtWm1iGTaK7XYzssdMMXgZVHnsgqaAFS3kznMFEA=; b=vxNIsOKWzaBS9ra4JTvhogoM4uesGOuh9j2eI59NVvGWa6h/W985vLjkYnjMX6ORTT iFnjoO+rmOa8N2HTWBd/enmqED4s8ux8xVZ80BD3fiMOD3SEsRzpdCqlVd7iMefu7AlK P0TBaL60BaNuHMQLTlcg9hksp1/Y78f7FhzRq5v7FYBZDdrg10KHUBYK9Dr/dUYAFj3b HZF51pwTKNKbgrm5xmLMZiAL7nkkX2k2oFvdd+ecpPs7RB4UiZkYRFv7ugAyhN1tVcDB IjFdtdf6UfBR8spiFZChzeMZEddebCvHqd/SS93a0+ERsQxcNJQCRiY4/Mh99C01j8yr X37Q== X-Gm-Message-State: AFqh2koiy2qQw6RTrsYFDV8Hj705r7HL7FICHWen81pQ9iC9RLE1nC2I h70SuOLNpmNZRJutXDgbUhg= X-Google-Smtp-Source: AMrXdXtCrOpTOIXnqtvQiMQt1xQE46H9qxE/PMCzdC55OKumJTn9GnX7xKApfCKs1HkgLcxv/0StFg== X-Received: by 2002:a5d:4b11:0:b0:242:29e9:184c with SMTP id v17-20020a5d4b11000000b0024229e9184cmr24103953wrq.29.1672667895995; Mon, 02 Jan 2023 05:58:15 -0800 (PST) Received: from localhost ([2a01:4c8:469:341:d1e1:a149:58ed:f096]) by smtp.gmail.com with ESMTPSA id r15-20020a0560001b8f00b002709e616fa2sm28560707wru.64.2023.01.02.05.58.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Jan 2023 05:58:15 -0800 (PST) Date: Mon, 2 Jan 2023 13:58:14 +0000 From: Lorenzo Stoakes To: David Hildenbrand Cc: linux-mm@kvack.org, Andrew Morton , Shuah Khan , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Vlastimil Babka , Jakub Matena , Matthew Wilcox , Mel Gorman , Michal Hocko Subject: Re: [PATCH] selftest/vm: add mremap expand merge offset test Message-ID: References: <20221216214436.405071-1-lstoakes@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 8D93720007 X-Stat-Signature: q4idt7cwrtbxp3nn4c6947fpccrhcpeh X-HE-Tag: 1672667897-56127 X-HE-Meta: U2FsdGVkX1/Tuo5xTcBkXXtqyvIBf++vCCWrw2G4xPRQOdNg/TM4uwyqt3YSAKrYorEQJLePXPtdHYH1ogxIYD15h1q+OnVuWhmFUlXKswt4MLUOBtMwi+SP4e9f/YvFiGSzrkn6X+dJ0ihFcs7CrnUDdCrQazU7Zxmi4uRqFUv1mYyfUb3AMLmQdYc4lT0m1oVrVm9FlI1fCe2MN4jhGVV3kfLpB6wnBtaBTnprCUUzA460aUXRCW7wJS37tfoweT/ZWWBvfXQ97ESHz+sXOW8fLCPbI9CN5a5YlECCfZv9XjUk31nngA28B0wy6DdieaP4EbQ7yinrG1JYSm8et4diB96myGe7PLTdRjCjyWVhcubn1KJP4QAQMHcuHdtJOiijCR5YP6WElWyv7n06KWfXI15hAdYmuAIECT/WWLafFiXWX+ghFppFzx59f1TD0+J9ma9KZ7IXit+q0XtenrATnbh9NOq50dMQwOf064bGy1gCIW3mnUu0DkT8bIWlNvs6hmZL8J8z1XVdKtqW91lN+8rFGDAWGjyiynCoL9a29mrFhIJRZpITOyQWrapkc7FpxTxU5fYcntCStUx8IVuKiLrq9sYf/Bq4HgKg08mg3UoqlA2tuA9t0Z2SiyeNN+8uYHsw21RcO7R2wj+x0Eb6Paet+mIW19KoFPCdm4q/mSkmtQkYZpaxuxJZtYEiwKlY2bGzAiwsR+DeQszGtRQ4n8DM3y9rdz75+KnvucBR7DZru3JbG6n4+cmX+F+1D+TaCIS+KR4eXYAv5BeVDLpdpuKmWeLVmfCJjZPZCBQdvde5ATxOI5SWXJ6FfscMvZPkGYVpen9QNp/OK/98N6OcOZdKgqh2r1ldpx9YT8qpi5Kc4zG8xsjVgUc/M9fD90rvRr6b0YKqizNBRrSpEOA4Ttv5nASWMvFT7QMifrhUiaeALBb0j7P5+f8igKbvw1c4yom5fc+fcbscKl6 g/BGryGE MXv2mjd23+PN1mO0t4yjrVods6U66nXCB+gz8WMORn3WmIuTIRLF+M8qjTh+k3LSOWYkWWMe4wA/88b7TYMxQQUGZRA== 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 Mon, Jan 02, 2023 at 02:32:55PM +0100, David Hildenbrand wrote: > On 16.12.22 22:44, Lorenzo Stoakes wrote: > > Add a test to assert that we can mremap() and expand a mapping starting > > from an offset within an existing mapping. We unmap the last page in a 3 > > page mapping to ensure that the remap should always succeed, before > > remapping from the 2nd page. > > > > This is additionally a regression test for the issue solved in "mm, mremap: > > fix mremap() expanding vma with addr inside vma" and confirmed to fail > > prior to the change and pass after it. > > > > Finally, this patch updates the existing mremap expand merge test to check > > error conditions and reduce code duplication between the two tests. > > > > Signed-off-by: Lorenzo Stoakes > > --- > > tools/testing/selftests/vm/mremap_test.c | 111 +++++++++++++++++++---- > > 1 file changed, 93 insertions(+), 18 deletions(-) > > > > diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c > > index 9496346973d4..28a17d4e8afd 100644 > > --- a/tools/testing/selftests/vm/mremap_test.c > > +++ b/tools/testing/selftests/vm/mremap_test.c > > @@ -119,30 +119,19 @@ static unsigned long long get_mmap_min_addr(void) > > } > > > > /* > > - * This test validates that merge is called when expanding a mapping. > > - * Mapping containing three pages is created, middle page is unmapped > > - * and then the mapping containing the first page is expanded so that > > - * it fills the created hole. The two parts should merge creating > > - * single mapping with three pages. > > + * Using /proc/self/maps, assert that the specified address range is contained > > + * within a single mapping. > > */ > > -static void mremap_expand_merge(unsigned long page_size) > > +static bool is_range_mapped(void *start, void *end) > > { > > - char *test_name = "mremap expand merge"; > > FILE *fp; > > char *line = NULL; > > size_t len = 0; > > bool success = false; > > - char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE, > > - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > > - > > - munmap(start + page_size, page_size); > > - mremap(start, page_size, 2 * page_size, 0); > > > > fp = fopen("/proc/self/maps", "r"); > > - if (fp == NULL) { > > - ksft_test_result_fail("%s\n", test_name); > > - return; > > - } > > + if (fp == NULL) > > + return false; > > This is unexpected. It would be valuable to ksft_print_msg("[INFO] .." > something, indicating that we don't know because we cannot access that info. > > ksft_print_msg("[INFO] Opening /proc/self/maps failed" > > > But I'd even suggest opening "/proc/self/maps" once in main() and failing > directly there. Then we don't have to worry about it here. > > > > > while (getline(&line, &len, fp) != -1) { > > char *first = strtok(line, "- "); > > @@ -150,16 +139,101 @@ static void mremap_expand_merge(unsigned long page_size) > > char *second = strtok(NULL, "- "); > > void *second_val = (void *) strtol(second, NULL, 16); > > > > - if (first_val == start && second_val == start + 3 * page_size) { > > + if (first_val <= start && second_val >= end) { > > success = true; > > break; > > } > > } > > + > > + fclose(fp); > > + return success; > > +} > > + > > +/* > > + * This test validates that merge is called when expanding a mapping. > > + * Mapping containing three pages is created, middle page is unmapped > > + * and then the mapping containing the first page is expanded so that > > + * it fills the created hole. The two parts should merge creating > > + * single mapping with three pages. > > + */ > > +static void mremap_expand_merge(unsigned long page_size) > > +{ > > + char *test_name = "mremap expand merge"; > > + bool success = false; > > + int errsv = 0; > > + char *remap; > > + char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE, > > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > > I'd suggest > > > char *remap, *start; > > start = mmap() > if (start == MAP_FAILED) { ... > > to make this easier to read. > > > + > > + if (start == MAP_FAILED) { > > + errsv = errno; > > + goto error; > > + } > > + > > + munmap(start + page_size, page_size); > > + remap = mremap(start, page_size, 2 * page_size, 0); > > + if (remap == MAP_FAILED) { > > + errsv = errno; > > + munmap(start, page_size); > > + munmap(start + 2 * page_size, page_size); > > + goto error; > > + } > > + > > + success = is_range_mapped(start, start + 3 * page_size); > > + > > + munmap(start, 3 * page_size); > > + goto out; > > + > > +error: > > + ksft_print_msg("Unexpected mapping/remapping error: %s\n", > > + strerror(errsv)); > > Please avoid the "error" label and just print proper errors directly at the > two callsites. Then, remove the "goto out". > > > +out: > > + if (success) > > + ksft_test_result_pass("%s\n", test_name); > > + else > > + ksft_test_result_fail("%s\n", test_name); > > +} > > + > > +/* > > + * Similar to mremap_expand_merge() except instead of removing the middle page, > > + * we remove the last then attempt to remap offset from the second page. This > > + * should result in the mapping being restored to its former state. > > + */ > > +static void mremap_expand_merge_offset(unsigned long page_size) > > +{ > > + > > + char *test_name = "mremap expand merge offset"; > > + bool success = false; > > + int errsv = 0; > > + char *remap; > > + char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE, > > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > > Dito. > > > + > > + if (start == MAP_FAILED) { > > + errsv = errno; > > + goto error; > > + } > > + > > + /* Unmap final page to ensure we have space to expand. */ > > + munmap(start + 2 * page_size, page_size); > > + remap = mremap(start + page_size, page_size, 2 * page_size, 0); > > + if (remap == MAP_FAILED) { > > + errsv = errno; > > + munmap(start, 2 * page_size); > > + goto error; > > + } > > + > > + success = is_range_mapped(start, start + 3 * page_size); > > + goto out; > > + > > +error: > > + ksft_print_msg("Unexpected mapping/remapping error: %s\n", > > + strerror(errsv)); > > Dito. > > > +out: > > > -- > Thanks, > > David / dhildenb > Ack on all, will spin a v2. Thanks for review!