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 E0FCEC27C53 for ; Fri, 7 Jun 2024 06:23:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 536716B0098; Fri, 7 Jun 2024 02:23:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E6526B009C; Fri, 7 Jun 2024 02:23:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3AEDD6B009D; Fri, 7 Jun 2024 02:23:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 1E9276B0098 for ; Fri, 7 Jun 2024 02:23:16 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B1C1D802E2 for ; Fri, 7 Jun 2024 06:23:15 +0000 (UTC) X-FDA: 82203100350.28.2DE3E41 Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by imf23.hostedemail.com (Postfix) with ESMTP id D061D14000E for ; Fri, 7 Jun 2024 06:23:12 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=zssYCzMg; spf=pass (imf23.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=quarantine) header.from=collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717741393; 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=9NFFdoo1AR4kpyuXDs4vNQSh1i3T75AcGui71Ke1OgI=; b=pKbgXu54RX9PLXZPdMG5I/oh67VWCEAsfnSTfL3MXljK+MiqaW/9hOEECQR7upBvnTOgaH L7rDkFcyayFh3QQnflQF7fubWmaa2RTt6JVFQ75VohdqsJ7FcecXeGj2TQwqlwpFvPm8Mx 2ZaRUs+Ffcx0G3vB2zfQjsDx7Sj1PPs= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=zssYCzMg; spf=pass (imf23.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.194 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=quarantine) header.from=collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717741393; a=rsa-sha256; cv=none; b=OJ5osfw66QCPQJaFdTONyUWKYW9EnetOYE/NlXBnd6sy5Rzfc0SBCJVoW6Vlw3LtqVQwnv u6uG6igKhEym6ATbseQGQLW8yrgaN80vmTVELaqZAYh8EhoHbS5wOoE5YdMQ/lDoZGNQav ifzps/rPuqpKjJXjYiiFCmLDyPMY4zg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1717741391; bh=e2/8FFik4VTYyTTf7c/nXYCFl6qwMjnndsppTg6LJ7E=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=zssYCzMg8cUZIbxN5Hk1doFPXNhnXXuLnyj4LV3PsHSaekKBP8nnqUsqpd7YquZ8t Vyb/87TR6JPRBaC1Yv/nIZhmo5RHMIWgf41l0TjA6QjeecEbL0EGP43C+Yw2b9WB+a 4gugNBJmGPTfM4rfO7JR83VWori0Qe4Rqa+a/x3T1HJEMChRVRLVxQkIXIuG1/EJbM BL7rTZ/D6Fk5eulMnjRx0d/gQ73XJpBwft7TZ1pByjyBkrWPq096CXN1/KblcbnHt0 9NNKYYNFPzv1YHQThyLVeM+wRlP4Ad4PNiFpRSxqZSXmeX57CuBhskNPFTCxNGeJqE cY02QuV2101iw== Received: from [100.113.15.66] (ec2-34-240-57-77.eu-west-1.compute.amazonaws.com [34.240.57.77]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: usama.anjum) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 384303780EC6; Fri, 7 Jun 2024 06:23:06 +0000 (UTC) Message-ID: <5ce292b6-179c-48e0-9079-ea07defbe178@collabora.com> Date: Fri, 7 Jun 2024 11:23:42 +0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: Muhammad Usama Anjum , linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Ritesh Harjani , Mike Rapoport , Muchun Song , David Hildenbrand , p.raghav@samsung.com Subject: Re: [PATCH v3] selftest: mm: Test if hugepage does not get leaked during __bio_release_pages() To: Donet Tom , Andrew Morton , Shuah Khan , Matthew Wilcox , Tony Battersby References: <20240607055046.138650-1-donettom@linux.ibm.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: <20240607055046.138650-1-donettom@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D061D14000E X-Stat-Signature: r157ch5zgeo6rib1xkm8gss8aeadcwpt X-Rspam-User: X-HE-Tag: 1717741392-195390 X-HE-Meta: U2FsdGVkX1+ZeO0bgckdaNc9H5jvyiCq0O5KyPXajlH6htgF3WtXMHSPNK4QQSlfKVRNKVBavfYJ/OJhtfNkEHIL+LK5AlMg50MEj+0lJgO9pAE0SNCUuJvHKLnNLEsLhl1n7pO1yboHzfef/WQWNV5cUUj1DLuT+mvByk82+0CbnRYgft8A1xCbru0JKx8afRxbw3bMVvw990mObdSgO0V6n2bbS+FrZUhYtg5NVg8MwSadHUDFnl36rQRliEqrCap62DZ0b/dYjpGqpTq3eKsZ54C3QwmbBNa6uMIYQ7nh0JkxCovywfonAKkugeODyHZbLv9F0fuQqQQ5lvgl14WvgYokS7iN6zY7er1ufkyj29kmet0gn6q9qTDGDNk1CPMrTk4ACFmYu8p/DQQuXfRBL+mp1bsIjwFY6shTNnMj6CV6t4fkVzhnMYoLD4MApCg6X2KNvMbUg4RgyNC76sLHKekvTgzNMqi+E5mCzyT5OA2L8almEXQv+TaIZJmRd8/SXA7fRzfc2h+8HcgaIoqCNeFpAh9EN9Mg18LG2pqn4y8fQ7vkzF/0ldsTzOMurp+D5+cpMSTiren0Es8Gq9gjpgj927T7beO6GO8zHmvrjkmcyH1YYbbjwTUXOCOUzV98T0q3w8U3ts35D1ONVnG29TSvV2VkGe83DCEO2aPzourbMxA1v3op/IPF41/LG1DuWVHey/lZFoms7A2VA8SdF/9ItQwtrJvSBh2IXtImUkMz/qd/4S5uKzv9YbH1CUPW9cxZ4uvFCf/7mGOYGf/aegud87bTJOFVdtkTCn+Cw7uWKDGKcZdCHzmbiScQsVvilWYa3713DZKbxEyILPAu7O6DhpzyatDooVzCjxlabOIeyZ5JxDJQQiLVIFn0bGCdQrXLF53IxOSV1EAY8C9nU9Q3mNYIwABfOA7VwXl5NLuiLAsVcdx6f97ZQtkBFzf1OHH8P488wUu3/yV mwfozEBJ r0q/JEbVHH55qrSTZbCaF2eKOpFc9vMIZ652P5dtEq/4O0hv+SrJNTng0RlGfMAu/Ef04BkfXY6/p97k0ToXGf0Nx89Rzi5/zGjN5PPZwoK8VMgQm8qPyJVLzUhY+aYMXw2KZiOfcN6vUIz5Qu6s1TPFgSewdw8KXDv95pkrx88LW2nYLZwO+fUm+WXgRZfJEPX4kVKFB/lOR94qbz91QGp4TYvXvylcS3zsK6ORpER26mkcxEj7rPBty7tXJwOLQav0HidK9oE5JvqCNJNwT5Kh04VPyGD4fDwgIPUEkRC+ZD/6XUx8kL+U7VjLH9645qC4wWv7YgHPUSx2rZ3FxY3oaNqgFAsFkDwITnan9ghJIh59eoQjX7B9FMog1oKFqdzC0tC4qT+jwqmfomZECbcQJBszfsbAH/MvcbB6mPJP4M1h4tjYGUDsszC505UD2Au22Wkra76MbAB/WmHzKofW3UiGkP61T+KyPxEUTxZzl3+67uUHUcLUWydoLqX4jhEzjL+sOvwpH72bMbnx0Y8M1b3bHqzIgs7rObla5WH1Klc6bxkEjNf91EnrZPH3t6FEUgsev2RT0lQJz6qPilYTB4eEGO6sy15uOcb8Ze5KU/t0= 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: List-Subscribe: List-Unsubscribe: On 6/7/24 10:50 AM, Donet Tom wrote: > Commit 1b151e2435fc ("block: Remove special-casing of compound > pages") caused a change in behaviour when releasing the pages > if the buffer does not start at the beginning of the page. This > was because the calculation of the number of pages to release > was incorrect. > This was fixed by commit 38b43539d64b ("block: Fix page refcounts > for unaligned buffers in __bio_release_pages()"). > > We pin the user buffer during direct I/O writes. If this buffer is a > hugepage, bio_release_page() will unpin it and decrement all references > and pin counts at ->bi_end_io. However, if any references to the hugepage > remain post-I/O, the hugepage will not be freed upon unmap, leading > to a memory leak. > > This patch verifies that a hugepage, used as a user buffer for DIO > operations, is correctly freed upon unmapping, regardless of whether > the offsets are aligned or unaligned w.r.t page boundary. > > Test Result Fail Scenario (Without the fix) > -------------------------------------------------------- > []# ./hugetlb_dio > TAP version 13 > 1..4 > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 1 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 2 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 3 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 6 > not ok 4 : Huge pages not freed! > Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0 > > Test Result PASS Scenario (With the fix) > --------------------------------------------------------- > []#./hugetlb_dio > TAP version 13 > 1..4 > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 1 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 2 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 3 : Huge pages freed successfully ! > No. Free pages before allocation : 7 > No. Free pages after munmap : 7 > ok 4 : Huge pages freed successfully ! > Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0 > > V3: > - Fixed the build error when it is compiled with _FORTIFY_SOURCE. > > V2: > - Addressed all review commets from Muhammad Usama Anjum > https://lore.kernel.org/all/20240604132801.23377-1-donettom@linux.ibm.com/ > > V1: > https://lore.kernel.org/all/20240523063905.3173-1-donettom@linux.ibm.com/#t > > Signed-off-by: Donet Tom > Co-developed-by: Ritesh Harjani (IBM) > Signed-off-by: Ritesh Harjani (IBM) > --- > tools/testing/selftests/mm/Makefile | 1 + > tools/testing/selftests/mm/hugetlb_dio.c | 118 +++++++++++++++++++++++ Missed my feedback on adding the test to vm_test.sh Other than this, LGTM. Please add following tag after the above change: Reviewed-by: Muhammad Usama Anjum > 2 files changed, 119 insertions(+) > create mode 100644 tools/testing/selftests/mm/hugetlb_dio.c > > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile > index 3b49bc3d0a3b..a1748a4c7df1 100644 > --- a/tools/testing/selftests/mm/Makefile > +++ b/tools/testing/selftests/mm/Makefile > @@ -73,6 +73,7 @@ TEST_GEN_FILES += ksm_functional_tests > TEST_GEN_FILES += mdwe_test > TEST_GEN_FILES += hugetlb_fault_after_madv > TEST_GEN_FILES += hugetlb_madv_vs_map > +TEST_GEN_FILES += hugetlb_dio > > ifneq ($(ARCH),arm64) > TEST_GEN_FILES += soft-dirty > diff --git a/tools/testing/selftests/mm/hugetlb_dio.c b/tools/testing/selftests/mm/hugetlb_dio.c > new file mode 100644 > index 000000000000..986f3b6c7f7b > --- /dev/null > +++ b/tools/testing/selftests/mm/hugetlb_dio.c > @@ -0,0 +1,118 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This program tests for hugepage leaks after DIO writes to a file using a > + * hugepage as the user buffer. During DIO, the user buffer is pinned and > + * should be properly unpinned upon completion. This patch verifies that the > + * kernel correctly unpins the buffer at DIO completion for both aligned and > + * unaligned user buffer offsets (w.r.t page boundary), ensuring the hugepage > + * is freed upon unmapping. > + */ > + > +#define _GNU_SOURCE > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "vm_util.h" > +#include "../kselftest.h" > + > +void run_dio_using_hugetlb(unsigned int start_off, unsigned int end_off) > +{ > + int fd; > + char *buffer = NULL; > + char *orig_buffer = NULL; > + size_t h_pagesize = 0; > + size_t writesize; > + int free_hpage_b = 0; > + int free_hpage_a = 0; > + const int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB; > + const int mmap_prot = PROT_READ | PROT_WRITE; > + > + writesize = end_off - start_off; > + > + /* Get the default huge page size */ > + h_pagesize = default_huge_page_size(); > + if (!h_pagesize) > + ksft_exit_fail_msg("Unable to determine huge page size\n"); > + > + /* Open the file to DIO */ > + fd = open("/tmp", O_TMPFILE | O_RDWR | O_DIRECT, 0664); > + if (fd < 0) > + ksft_exit_fail_perror("Error opening file\n"); > + > + /* Get the free huge pages before allocation */ > + free_hpage_b = get_free_hugepages(); > + if (free_hpage_b == 0) { > + close(fd); > + ksft_exit_skip("No free hugepage, exiting!\n"); > + } > + > + /* Allocate a hugetlb page */ > + orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0); > + if (orig_buffer == MAP_FAILED) { > + close(fd); > + ksft_exit_fail_perror("Error mapping memory\n"); > + } > + buffer = orig_buffer; > + buffer += start_off; > + > + memset(buffer, 'A', writesize); > + > + /* Write the buffer to the file */ > + if (write(fd, buffer, writesize) != (writesize)) { > + munmap(orig_buffer, h_pagesize); > + close(fd); > + ksft_exit_fail_perror("Error writing to file\n"); > + } > + > + /* unmap the huge page */ > + munmap(orig_buffer, h_pagesize); > + close(fd); > + > + /* Get the free huge pages after unmap*/ > + free_hpage_a = get_free_hugepages(); > + > + /* > + * If the no. of free hugepages before allocation and after unmap does > + * not match - that means there could still be a page which is pinned. > + */ > + if (free_hpage_a != free_hpage_b) { > + ksft_print_msg("No. Free pages before allocation : %d\n", free_hpage_b); > + ksft_print_msg("No. Free pages after munmap : %d\n", free_hpage_a); > + ksft_test_result_fail(": Huge pages not freed!\n"); > + } else { > + ksft_print_msg("No. Free pages before allocation : %d\n", free_hpage_b); > + ksft_print_msg("No. Free pages after munmap : %d\n", free_hpage_a); > + ksft_test_result_pass(": Huge pages freed successfully !\n"); > + } > +} > + > +int main(void) > +{ > + size_t pagesize = 0; > + > + ksft_print_header(); > + ksft_set_plan(4); > + > + /* Get base page size */ > + pagesize = psize(); > + > + /* start and end is aligned to pagesize */ > + run_dio_using_hugetlb(0, (pagesize * 3)); > + > + /* start is aligned but end is not aligned */ > + run_dio_using_hugetlb(0, (pagesize * 3) - (pagesize / 2)); > + > + /* start is unaligned and end is aligned */ > + run_dio_using_hugetlb(pagesize / 2, (pagesize * 3)); > + > + /* both start and end are unaligned */ > + run_dio_using_hugetlb(pagesize / 2, (pagesize * 3) + (pagesize / 2)); > + > + ksft_finished(); > +} > + -- BR, Muhammad Usama Anjum