From patchwork Thu Aug 29 14:10:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 1978466 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=G+RmxxIJ; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.linux.it (client-ip=2001:1418:10:5::2; helo=picard.linux.it; envelope-from=ltp-bounces+incoming=patchwork.ozlabs.org@lists.linux.it; receiver=patchwork.ozlabs.org) Received: from picard.linux.it (picard.linux.it [IPv6:2001:1418:10:5::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WvjqC2Zv9z1yZ9 for ; Fri, 30 Aug 2024 00:10:31 +1000 (AEST) Received: from picard.linux.it (localhost [IPv6:::1]) by picard.linux.it (Postfix) with ESMTP id 8E9053D2815 for ; Thu, 29 Aug 2024 16:10:28 +0200 (CEST) X-Original-To: ltp@lists.linux.it Delivered-To: ltp@picard.linux.it Received: from in-4.smtp.seeweb.it (in-4.smtp.seeweb.it [IPv6:2001:4b78:1:20::4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by picard.linux.it (Postfix) with ESMTPS id 956A63D2808 for ; Thu, 29 Aug 2024 16:10:25 +0200 (CEST) Authentication-Results: in-4.smtp.seeweb.it; spf=pass (sender SPF authorized) smtp.mailfrom=redhat.com (client-ip=170.10.133.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=david@redhat.com; receiver=lists.linux.it) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by in-4.smtp.seeweb.it (Postfix) with ESMTPS id 4A46F100287D for ; Thu, 29 Aug 2024 16:10:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1724940620; h=from:from: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; bh=7UPA9wwtp/mSKxKmkTeukZQiBLyuNpFLTqQQcPJJHOg=; b=G+RmxxIJNezLpuRjPZE+jdcXu1CoM8hYnG9gWi6EEAfa7Vy5Y36mJSBSqtRbW52S8+GiQ1 UKEDj9uiv+mN5VSlwGfwBlRuoBUjQf8cbuGuqhfiLpcUxQPFcEeQqZB5hZY7nLu7QzHFAq /WLTIg+RcVORBX8lnxxCZktwpsBhWlc= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-428-j0lrGNIXPI2Q8P0sNlw3ZQ-1; Thu, 29 Aug 2024 10:10:18 -0400 X-MC-Unique: j0lrGNIXPI2Q8P0sNlw3ZQ-1 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id B0DD6191815B; Thu, 29 Aug 2024 14:10:16 +0000 (UTC) Received: from t14s.redhat.com (unknown [10.39.193.245]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 8D8E119560A3; Thu, 29 Aug 2024 14:10:14 +0000 (UTC) From: David Hildenbrand To: ltp@lists.linux.it Date: Thu, 29 Aug 2024 16:10:02 +0200 Message-ID: <20240829141002.1962303-1-david@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=0.1 required=7.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=disabled version=4.0.0 X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on in-4.smtp.seeweb.it X-Virus-Scanned: clamav-milter 1.0.3 at in-4.smtp.seeweb.it X-Virus-Status: Clean Subject: [LTP] [PATCH v1] move_pages04: check for "invalid area", "no page mapped" and "shared zero page mapped" X-BeenThere: ltp@lists.linux.it X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Test Project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Hildenbrand Errors-To: ltp-bounces+incoming=patchwork.ozlabs.org@lists.linux.it Sender: "ltp" While the kernel commit d899844e9c98 ("mm: fix status code which move_pages() returns for zero page") fixed the return value when the shared zero page was encountered to match what was state in the man page, it unfortunately also changed the behavior when no page is mapped yet -- when no page was faulted in/populated on demand. Then, this test started failing, and we thought we would be testing for the "zero page" case, but actually we were testing for the "no page mapped" case, and didn't realize that the kernel commit had unintended side effects. As we are changing the behavior back to return "-ENOENT" in the kernel for the "no page mapped" case, while still keeping the "shared zero page" case to return "-EFAULT" the test starts failing again ... The man page clearly spells out that the expectation for the zero page is "-EFAULT", and that "-EFAULT" can also be returned if "the memory area is not mapped by the process" -- which means that there is no VMA/mmap() covering that address. The man page also documents that "-ENOENT" is returned when "The page is not present", which includes "there is nothing mapped". So let's fix the test and properly testing for all three separate things: "invalid area/page", "no page mapped" and "shared zero page mapped"> Cc: Cyril Hrubis Signed-off-by: David Hildenbrand --- The kernel change[1] that will revert to the documented behavior -- return -ENOENT when no page is mapped yet -- is not upstream yet, but the assumption is that it will go upstream in the next merge window, to land in v6.12. Consequently, this test will now fail (as expected) on kernels between v4.3 and current mainline. We seemed to have "hacked" the test to make kernels < 4.3 still pass, even though they were handling zero pages the wrong way. Should we add a similar "hack" for kernels >= 4.3 up to the one where the kernel behavior will change? (likely v6.12)? On current kernels: #./move_pages04 move_pages04 1 TFAIL : move_pages04.c:184: status[1] is EFAULT, expected ENOENT move_pages04 2 TPASS : status[2] has expected value move_pages04 3 TPASS : status[3] has expected value On kernels with [1]: # ./move_pages04 move_pages04 1 TPASS : status[1] has expected value move_pages04 2 TPASS : status[2] has expected value move_pages04 3 TPASS : status[3] has expected value Note that I tried converting this test to the new API, but the shared code move_pages_support.c also till uses the old API, so it's not as straight-forward as it seems. [1 https://lkml.kernel.org/r/20240802155524.517137-5-david@redhat.com --- .../kernel/syscalls/move_pages/move_pages04.c | 104 ++++++++++++++---- 1 file changed, 83 insertions(+), 21 deletions(-) diff --git a/testcases/kernel/syscalls/move_pages/move_pages04.c b/testcases/kernel/syscalls/move_pages/move_pages04.c index f53453ab4..7a20a1328 100644 --- a/testcases/kernel/syscalls/move_pages/move_pages04.c +++ b/testcases/kernel/syscalls/move_pages/move_pages04.c @@ -26,15 +26,24 @@ * move_pages04.c * * DESCRIPTION - * Failure when page does not exit. + * Failure when the memory area is not valid, no page is mapped yet or + * the shared zero page is mapped. * * ALGORITHM * - * 1. Pass zero page (allocated, but not written to) as one of the - * page addresses to move_pages(). - * 2. Check if the corresponding status is set to: + * 1. Pass the address of a valid memory area where no where no page is + * mapped yet (not read/written), the address of a valid memory area + * where the shared zero page is mapped (read, but not written to) + * and the address of an invalid memory area as page addresses to + * move_pages(). + * 2. Check if the corresponding status for "no page mapped" is set to + * -ENOENT. Note that [1] changed the behavior to return -EFAULT by + * accident. + * 3. Check if the corresponding status for "shared zero page" is set to: * -ENOENT for kernels < 4.3 * -EFAULT for kernels >= 4.3 [1] + * 4. Check if the corresponding status for "invalid memory area" is set + * to -EFAULT. * * [1] * d899844e9c98 "mm: fix status code which move_pages() returns for zero page" @@ -64,10 +73,12 @@ #include "test.h" #include "move_pages_support.h" -#define TEST_PAGES 2 +#define TEST_PAGES 4 #define TEST_NODES 2 #define TOUCHED_PAGES 1 -#define UNTOUCHED_PAGE (TEST_PAGES - 1) +#define NO_PAGE TOUCHED_PAGES +#define ZERO_PAGE (NO_PAGE + 1) +#define INVALID_PAGE (ZERO_PAGE + 1) void setup(void); void cleanup(void); @@ -89,12 +100,12 @@ int main(int argc, char **argv) int lc; unsigned int from_node; unsigned int to_node; - int ret, exp_status; + int ret, exp_zero_page_status; if ((tst_kvercmp(4, 3, 0)) >= 0) - exp_status = -EFAULT; + exp_zero_page_status = -EFAULT; else - exp_status = -ENOENT; + exp_zero_page_status = -ENOENT; ret = get_allowed_nodes(NH_MEMS, 2, &from_node, &to_node); if (ret < 0) @@ -106,6 +117,7 @@ int main(int argc, char **argv) int nodes[TEST_PAGES]; int status[TEST_PAGES]; unsigned long onepage = get_page_size(); + char tmp; /* reset tst_count in case we are looping */ tst_count = 0; @@ -114,14 +126,44 @@ int main(int argc, char **argv) if (ret == -1) continue; - /* Allocate page and do not touch it. */ - pages[UNTOUCHED_PAGE] = numa_alloc_onnode(onepage, from_node); - if (pages[UNTOUCHED_PAGE] == NULL) { - tst_resm(TBROK, "failed allocating page on node %d", + /* + * Allocate memory and do not touch it. Consequently, no + * page will be faulted in / mapped into the page tables. + */ + pages[NO_PAGE] = numa_alloc_onnode(onepage, from_node); + if (pages[NO_PAGE] == NULL) { + tst_resm(TBROK, "failed allocating memory on node %d", from_node); goto err_free_pages; } + /* + * Allocate memory, read from it, but do not write to it. This + * will populate the shared zeropage. + */ + pages[ZERO_PAGE] = numa_alloc_onnode(onepage, from_node); + if (pages[ZERO_PAGE] == NULL) { + tst_resm(TBROK, "failed allocating memory on node %d", + from_node); + goto err_free_pages; + } + /* Make the compiler not optimize-out the read. */ + tmp = *((char *)pages[ZERO_PAGE]); + asm volatile("" : "+r" (tmp)); + + /* + * Temporarily allocate memory and free it immediately. Do this + * as the last step so the area won't get reused before we're + * done. + */ + pages[INVALID_PAGE] = numa_alloc_onnode(onepage, from_node); + if (pages[INVALID_PAGE] == NULL) { + tst_resm(TBROK, "failed allocating memory on node %d", + from_node); + goto err_free_pages; + } + numa_free(pages[INVALID_PAGE], onepage); + for (i = 0; i < TEST_PAGES; i++) nodes[i] = to_node; @@ -135,20 +177,40 @@ int main(int argc, char **argv) tst_resm(TINFO, "move_pages() returned %d", ret); } - if (status[UNTOUCHED_PAGE] == exp_status) { + if (status[NO_PAGE] == -ENOENT) { tst_resm(TPASS, "status[%d] has expected value", - UNTOUCHED_PAGE); + NO_PAGE); } else { tst_resm(TFAIL, "status[%d] is %s, expected %s", - UNTOUCHED_PAGE, - tst_strerrno(-status[UNTOUCHED_PAGE]), - tst_strerrno(-exp_status)); + NO_PAGE, + tst_strerrno(-status[NO_PAGE]), + tst_strerrno(ENOENT)); + } + + if (status[ZERO_PAGE] == exp_zero_page_status) { + tst_resm(TPASS, "status[%d] has expected value", + ZERO_PAGE); + } else { + tst_resm(TFAIL, "status[%d] is %s, expected %s", + ZERO_PAGE, + tst_strerrno(-status[ZERO_PAGE]), + tst_strerrno(-exp_zero_page_status)); + } + + if (status[INVALID_PAGE] == -EFAULT) { + tst_resm(TPASS, "status[%d] has expected value", + INVALID_PAGE); + } else { + tst_resm(TFAIL, "status[%d] is %s, expected %s", + INVALID_PAGE, + tst_strerrno(-status[INVALID_PAGE]), + tst_strerrno(EFAULT)); } err_free_pages: - /* This is capable of freeing both the touched and - * untouched pages. - */ + /* Memory for the invalid page was already freed. */ + pages[INVALID_PAGE] = NULL; + /* This is capable of freeing all memory we allocated. */ free_pages(pages, TEST_PAGES); } #else