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 7F586EB64DC for ; Thu, 6 Jul 2023 05:17:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6976E8D0002; Thu, 6 Jul 2023 01:17:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 647228D0001; Thu, 6 Jul 2023 01:17:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 536778D0002; Thu, 6 Jul 2023 01:17:35 -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 4381A8D0001 for ; Thu, 6 Jul 2023 01:17:35 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 16603803CB for ; Thu, 6 Jul 2023 05:17:35 +0000 (UTC) X-FDA: 80980029270.24.28CB4D4 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by imf16.hostedemail.com (Postfix) with ESMTP id 164B218000E for ; Thu, 6 Jul 2023 05:17:32 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=Tcb6D60Q; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf16.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688620653; 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=1cWRvk1GXBbBHWuAirpNTmPQw35f3iu9HBmwFNZOiV4=; b=i7u36QoDqCeQEdjgxYmOb12hShUBDsPOCDNbsqFZiyjral9WPfbi4nLeeWM5M7DKC+/fR3 FR57EmGQuhV4yujdjC5C9jRvTJ4bXYrWR3WUFFY6H8G5AoUh0L9DCwKLUybU0mi/ise/Hw wR/SicakWoGeeeOlnpZBB+jitqnKTnA= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=Tcb6D60Q; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf16.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688620653; a=rsa-sha256; cv=none; b=gHdSZF5BQdvs7KFKRhzKrmrTtngDIK0bC+N/Qh4hVfpcidGoM4DwPS7m/aoFrVPHQK+HU+ +rm0y2KYOKERo6VqhtV112c76kH9a1WkwjcIZTwM/dDlB5/7Qkf2lwyBW6WZXzwrHPbJ5j SDdlNBzG4nKTZS7HZWuYD3YzezZuFnM= Received: from [192.168.10.54] (unknown [119.155.52.241]) (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 madras.collabora.co.uk (Postfix) with ESMTPSA id C747E660585C; Thu, 6 Jul 2023 06:17:23 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1688620651; bh=q4UGyIJqqzwWpndwRfGrl8koZCuNxBkVB317S/eWJaA=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=Tcb6D60QYOcj5jT42ZAoKAoPs/BG7HzZGCJvmbJrEJjvgRcJlknbvA0Mq/M+hCu0Y pi6Xf3o/l4ifELvZynibFSHdjsytNy4GRaD6+wtKAzgtgmS0Kmogxlb/IfMJsP0Slu OCYTQmbY8ogEddxWIvbUJI34bTF4lemBdDuBCQ7IhW/ISCQCiL6SoxoS1ip4Yp8FWp qAO75uESGswyMsKW33DQfb7eKM8+WgAquqRSmaftloKn2ag1UeMyQBp3+dNcwcbVDH iRtRFc2s415IDgJIsuqUoCTvS4l0LSBaUg+JCCAHfs/693bhXAzPy1IyrYl6gaziwl 6hUaQZmZQ6fGA== Message-ID: Date: Thu, 6 Jul 2023 10:17:19 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Cc: Muhammad Usama Anjum , Peter Xu , David Hildenbrand , Andrew Morton , =?UTF-8?B?TWljaGHFgiBNaXJvc8WC?= =?UTF-8?Q?aw?= , Danylo Mocherniuk , Paul Gofman , Cyrill Gorcunov , Mike Rapoport , Nadav Amit , Alexander Viro , Shuah Khan , Christian Brauner , Yang Shi , Vlastimil Babka , "Liam R . Howlett" , Yun Zhou , Suren Baghdasaryan , Alex Sierra , Matthew Wilcox , Pasha Tatashin , Axel Rasmussen , "Gustavo A . R . Silva" , Dan Williams , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Greg KH , kernel@collabora.com Subject: Re: [PATCH v22 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs To: Andrei Vagin References: <20230628095426.1886064-1-usama.anjum@collabora.com> <20230628095426.1886064-3-usama.anjum@collabora.com> <49015b82-2f7f-fa34-d56a-2f427f38f28f@collabora.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: hmwwmb1enj3icqj8o5fw39xyeq1jrk39 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 164B218000E X-HE-Tag: 1688620652-109037 X-HE-Meta: U2FsdGVkX18z43NZJ6rALp3PgMfcqsl8qrmOC9rDZNS086GUOZBVZdHJKlNz9JeuaDIIrZYmEHLNuPs+xZ9FlzBFa6QlxqbMdRgB7l0qEZSWMh6PmimjbhLPeS8GT4kipJSL0Xfzok1v0Mp/fpzGDGM+xZkntiZozT9lZJG7K2XHNLIXDRcfIlb4wg0S+n0KldyujpIUl0g+x4zBKZLmlbDGbUHS451xIg9Zy2WgyyFC7tOekCsaUGTnPkIrdqH8FXvIoJ20RP7lp7KFeeKCSH1gwFDMOGiu9JA2c7nhIOahygugIajKuf0Vhl9kesoHGMFncgYHCQ1uWT1DrtuZ68UoiE2oDyaaJE82Ycy9MAqu1k9zWtbuIv7O7RVdSnqolvxi6YQXB7dvTUO3QT9ONKl1eDsl5IDejkbHVgpFNhAohBuXzfTGQRpmeUcygh1HZvTn0bdxdBueFb8SKcnyzCPgjp+jT1bi+aOWrOTkAX0swDtuAtO/4tWd2uJgvYNqHSX2kCnf/rXD6WTmnWtmGCp6bSiOBxV/YDPHgKCwDRaHgUHnPvVIfDI6UbxfSzITX9VDZG6YIl58HBv1JNiWE3uzdNydOxvEtju18Wtaowih8RnJ+livYe2O8AvsikA6FfxsRO08ssaG8PQST6COXNorvIhDcLiyeg4NEwAVPk1AcssQQBxEk2bFys0iGDKwrDqmvZlEtjLN2lMnanUbQs5U1OMBj261ynYb6oBaqVd7iQYstupCoRry8rfU5eO8p3juF5MnmOdMUrMaKkTJNLttRzhpEAz4X3Trre7Y1l3vrsqcqxFxs+2h40f5n4YViesie6Y2VhYoP2KNBBqaDDRsoi7JkHWqBgcm8vCRckzU0ETQMvgGllYqzp6AohbmCaDLuUL7/TGIGpWnSnBYGzdJT2vhEPMXb/kLRUgD4FjVkZM8/yzciR7hrDylZJPa2QqR9Va9hmwhrnaf8vm B4LGQmUa LIkMRGFKmrfBHMXGA2+Ax6tqCfmzqbhuJbdXsK1yxoc9jr/zT0EwiZuy1lmy5jW65FVpfNyl5bFkEugQb1+T4/LuVgK6P55OMXMDWKDFTTSJE0CZw9cF3fos2RGzJ+x0GG2oEE7L9TSpiZ9xqt00+uibfLoXhTjpYg4P+4e/a63StOthfvSz1IgYlWSY1YzHWhv71K34NjDNTaROqFi7UqopxAHf2mkcYPyOsOeyjywGEKKo9jT22Yg8EWb6K/oYxFHV+AwsWVu+NKHAIgiqzoBiPmtv6EW0r7vAgdnwQTxY1JJCa/aNQCRNFeA== 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 7/3/23 8:07 PM, Andrei Vagin wrote: > On Mon, Jul 03, 2023 at 11:47:37AM +0500, Muhammad Usama Anjum wrote: >> On 6/30/23 8:01 PM, Andrei Vagin wrote: >>> On Wed, Jun 28, 2023 at 02:54:23PM +0500, Muhammad Usama Anjum wrote: >>>> This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear >>>> the info about page table entries. The following operations are supported >>>> in this ioctl: >>>> - Get the information if the pages have been written-to (PAGE_IS_WRITTEN), >>>> file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT), swapped >>>> (PAGE_IS_SWAPPED) or page has pfn zero (PAGE_IS_PFNZERO). >>>> - Find pages which have been written-to and/or write protect the pages >>>> (atomic PM_SCAN_OP_GET + PM_SCAN_OP_WP) >>>> >>>> This IOCTL can be extended to get information about more PTE bits. The >>>> entire address range passed by user [start, end) is scanned until either >>>> the user provided buffer is full or max_pages have been found. >>>> >>>> Signed-off-by: Muhammad Usama Anjum >>> >>> >>> >>>> + >>>> +static long do_pagemap_scan(struct mm_struct *mm, unsigned long __arg) >>>> +{ >>>> + struct pm_scan_arg __user *uarg = (struct pm_scan_arg __user *)__arg; >>>> + unsigned long long start, end, walk_start, walk_end; >>>> + unsigned long empty_slots, vec_index = 0; >>>> + struct mmu_notifier_range range; >>>> + struct page_region __user *vec; >>>> + struct pagemap_scan_private p; >>>> + struct pm_scan_arg arg; >>>> + int ret = 0; >>>> + >>>> + if (copy_from_user(&arg, uarg, sizeof(arg))) >>>> + return -EFAULT; >>>> + >>>> + start = untagged_addr((unsigned long)arg.start); >>>> + end = untagged_addr((unsigned long)arg.end); >>>> + vec = (struct page_region __user *)untagged_addr((unsigned long)arg.vec); >>>> + >>>> + ret = pagemap_scan_args_valid(&arg, start, end, vec); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + p.max_pages = (arg.max_pages) ? arg.max_pages : ULONG_MAX; >>>> + p.found_pages = 0; >>>> + p.required_mask = arg.required_mask; >>>> + p.anyof_mask = arg.anyof_mask; >>>> + p.excluded_mask = arg.excluded_mask; >>>> + p.return_mask = arg.return_mask; >>>> + p.flags = arg.flags; >>>> + p.flags |= ((p.required_mask | p.anyof_mask | p.excluded_mask) & >>>> + PAGE_IS_WRITTEN) ? PM_SCAN_REQUIRE_UFFD : 0; >>>> + p.cur_buf.start = p.cur_buf.len = p.cur_buf.flags = 0; >>>> + p.vec_buf = NULL; >>>> + p.vec_buf_len = PAGEMAP_WALK_SIZE >> PAGE_SHIFT; >>>> + >>>> + /* >>>> + * Allocate smaller buffer to get output from inside the page walk >>>> + * functions and walk page range in PAGEMAP_WALK_SIZE size chunks. As >>>> + * we want to return output to user in compact form where no two >>>> + * consecutive regions should be continuous and have the same flags. >>>> + * So store the latest element in p.cur_buf between different walks and >>>> + * store the p.cur_buf at the end of the walk to the user buffer. >>>> + */ >>>> + if (IS_PM_SCAN_GET(p.flags)) { >>>> + p.vec_buf = kmalloc_array(p.vec_buf_len, sizeof(*p.vec_buf), >>>> + GFP_KERNEL); >>>> + if (!p.vec_buf) >>>> + return -ENOMEM; >>>> + } >>>> + >>>> + if (IS_PM_SCAN_WP(p.flags)) { >>>> + mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_VMA, 0, >>>> + mm, start, end); >>>> + mmu_notifier_invalidate_range_start(&range); >>>> + } >>>> + >>>> + walk_start = walk_end = start; >>>> + while (walk_end < end && !ret) { >>>> + if (IS_PM_SCAN_GET(p.flags)) { >>>> + p.vec_buf_index = 0; >>>> + >>>> + /* >>>> + * All data is copied to cur_buf first. When more data >>>> + * is found, we push cur_buf to vec_buf and copy new >>>> + * data to cur_buf. Subtract 1 from length as the >>>> + * index of cur_buf isn't counted in length. >>>> + */ >>>> + empty_slots = arg.vec_len - vec_index; >>>> + p.vec_buf_len = min(p.vec_buf_len, empty_slots - 1); >>>> + } >>>> + >>>> + ret = mmap_read_lock_killable(mm); >>>> + if (ret) >>>> + goto return_status; >>>> + >>>> + walk_end = min((walk_start + PAGEMAP_WALK_SIZE) & PAGEMAP_WALK_MASK, end); >>>> + >>>> + ret = walk_page_range(mm, walk_start, walk_end, >>>> + &pagemap_scan_ops, &p); >>>> + mmap_read_unlock(mm); >>>> + >>>> + if (ret && ret != PM_SCAN_FOUND_MAX_PAGES && >>>> + ret != PM_SCAN_END_WALK) >>>> + goto return_status; >>>> + >>>> + walk_start = walk_end; >>>> + if (IS_PM_SCAN_GET(p.flags) && p.vec_buf_index) { >>>> + if (copy_to_user(&vec[vec_index], p.vec_buf, >>>> + p.vec_buf_index * sizeof(*p.vec_buf))) { >>>> + /* >>>> + * Return error even though the OP succeeded >>>> + */ >>>> + ret = -EFAULT; >>>> + goto return_status; >>>> + } >>>> + vec_index += p.vec_buf_index; >>>> + } >>>> + } >>>> + >>>> + if (p.cur_buf.len) { >>>> + if (copy_to_user(&vec[vec_index], &p.cur_buf, sizeof(p.cur_buf))) { >>>> + ret = -EFAULT; >>>> + goto return_status; >>>> + } >>>> + vec_index++; >>>> + } >>>> + >>>> + ret = vec_index; >>>> + >>>> +return_status: >>>> + arg.start = (unsigned long)walk_end; >>> >>> This doesn't look right. pagemap_scan_pmd_entry can stop early. For >>> example, it can happen when it hits the max_pages limit. Do I miss >>> something? >> The walk_page_range() calls pagemap_scan_pmd_entry(). So whatever status is >> returned from pagemap_scan_pmd_entry(), walk_page_range() returns to this >> function where we are handling the status code. After while loop starts, >> there is only 1 return path. Hence there isn't any path missing where we'll >> miss setting arg.start. > > I mean walk_end isn't actually the end address. The end adress should be > the next page after the last revised page. Here we don't know whether > all pages in [walk_start, walk_end) has been revised. Sorry, understood. Let me post the new patch series. > >> >>> >>>> + if (copy_to_user(&uarg->start, &arg.start, sizeof(arg.start))) >>>> + ret = -EFAULT; >>>> + >>>> + if (IS_PM_SCAN_WP(p.flags)) >>>> + mmu_notifier_invalidate_range_end(&range); >>>> + >>>> + kfree(p.vec_buf); >>>> + return ret; >>>> +} >>>> + >> >> -- >> BR, >> Muhammad Usama Anjum -- BR, Muhammad Usama Anjum