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 8E06FEB64DC for ; Mon, 3 Jul 2023 06:47:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E04E48E0096; Mon, 3 Jul 2023 02:47:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DB4038E007C; Mon, 3 Jul 2023 02:47:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7BD48E0096; Mon, 3 Jul 2023 02:47:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B43208E007C for ; Mon, 3 Jul 2023 02:47:54 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 812BF80843 for ; Mon, 3 Jul 2023 06:47:54 +0000 (UTC) X-FDA: 80969370468.08.574A789 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by imf02.hostedemail.com (Postfix) with ESMTP id 7707D8000D for ; Mon, 3 Jul 2023 06:47:52 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=If01X33w; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf02.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=1688366872; 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=dc7EWnnGaMrcE2gznjMqJ2tmrIjue/deajsjchZIZjQ=; b=gXVvxffm9sgr8x8q4bm9bWsEkFMGGc7PWnugMDOrldKZ3PwZ/ey+wIJJh6CObAsscE15GG R6LPub1o8/Qn0Tka7AhiuMZbMIQ0dWkHk0m9AwalkJr9wauCaNlnkRe4rP/+jlYfBVS63Q cEiNYghdH4KUfNJEQtjq/98pK4mmcVA= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=If01X33w; dmarc=pass (policy=quarantine) header.from=collabora.com; spf=pass (imf02.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=1688366872; a=rsa-sha256; cv=none; b=xW2RYZhNRmmGO+TrDjxFMg1H2yTdtDV8fVdkVkxFJLektUIuD4Vsl5d1gnKIrwdlggi0DS ZjndQ4Q5MYC415EisT7Q8FhRL5nV0CeD6KaxVPJcHncGtvFYXXrhapo3GXF5fA8BaePj3s FVGOy19HtAwlAz3DYDMj9WHoPWaGVIA= Received: from [192.168.43.110] (unknown [116.71.174.96]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: usama.anjum) by madras.collabora.co.uk (Postfix) with ESMTPSA id 24DBB660297B; Mon, 3 Jul 2023 07:47:41 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1688366870; bh=90Czpk7xjgiSE+C5Q1cQigH8jDccb9HqLzCtvzTM+Sg=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=If01X33wkula4geSXjruOsp6MgkIKUn5vFjpXkEzy9VuhpWCNIXTqj0Emqf4hOwbg f9pG7S3y6wo+Mrgfxm9VfwAlMSNZZAZ/6woKa/Gwyp1lBM8Rc83ACJyTxUoxqzBYLw /7eKs45mJ3ABS/VMQ+ENqPEnX64gflfhyb+Z/d4E+j0+pkXNA1x7WX+1rSX0vh+Ef1 FWhFBh4tXKO8w2pHOmrdtPL89PbjFzNkYoFUaEYD4ISYXlSho0lovHAwxfZRu21YGB fWemHqcmyzEwUnf3SZH089I2DBzOEmmfQgLYAj8uPgDiE3FnCfvlaJAiI1FkwNpOl0 FaMCwbsjxDrVg== Message-ID: <49015b82-2f7f-fa34-d56a-2f427f38f28f@collabora.com> Date: Mon, 3 Jul 2023 11:47:37 +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> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 7707D8000D X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: zeeczyzsjki7ixy4yp6469fg6wrk6kbw X-HE-Tag: 1688366872-919653 X-HE-Meta: U2FsdGVkX1/pSeeAVHwH+nZwVUIYahDTtoXUdGA85TwX+/8mEP33nTsXawVmIl/JaOwv3G8xlVg0Jt3ALV3WD7HctOfvPVXi7dSZ5w7Z5TD2ARhrF/0mfGz4bnM9iEm2+vut+jwOJ+UPJBdE24gV3yXUi5vJPNnN39JIhyPL7fTd7G6oEfMm0JRHqAuLXwZBco0XDWZYGU4L7OdWWL6lQMMvfUivMbwhdtXldH0h2R0ARCrFemgpdEN6eB9v3jjypGrfj4GkCutowGCZqifprWAC9jKKGOpN906ACJTia4Wvp2fmj2s2D1/bUTwH/tDcleGmgqBGYGoyL+Hx5ogOdsP3ChdpEGsUFuFEHXYdwWPIZckGlOCjpeheEnvPczB2yd4HZ28jakd8TrEXQNoonkQN/3WY22HU0qBEVU9RJpLKiMwmWoMkm9zQDruMrw8JhehPq/CD8YfQ7fgWrKKvqlW751b0ZQX92KX/w+ktlY1XGZ4GXJOuSzZJU2puDB/Ys0bOgMQXLZkBUDBd6dAQ/PLKfqSC+aEGzl5bGVJYdEK9x/UwbiMQll7QfMWpPwpteYprdcWEOu2AN+gsTIRJ4Wy5p2rqS6RUFgsyUgtoxhwpOb1VnQK/RHAXOpywp2ezWe6Zl+D+H/8+YzF5MALKg/nfTWszF9vvwo0arxyRgcFwhpFocGkOpK6vKOOZ24yHE6pHSvcISl+YHGmr2fC/aBKwKkfRc7OkMCwkdRa01QA7Ow8iDK/adtgeMc9gMsuC8zcTT9J3+ik33eyEqzE51NvhPR7Ph3r1Hiufr4Kks+PH6AGbL5kF3FtaQFpbMYLMMJR5L4Gq2W7j5QJI+5KrCvg+mX7yn/idaD2RwsNVix3+8iJ3fs1VngX0vPxJpdT4Ru2HMu6Q8TgOyyNPSKYs1qKSdDJMvpetaO5x/lp93opLUkoBm87VcSKtHlcn+Nfx/cKFtniBkunVGvVz4Lj y5y3biSF zVaAkmjGN0c6Ot//IGs27G56NF4zoNTu4CblQZ8V6WTJ99cLdvy+QX+fvcxo0UGo8LIHZjmeSmOlc7jv0ReC0KrYLnVWXsyyoQPK7rV5MGODcC7Zhkiug3rwB24RQ/siB4/He5T7xJ2CT4iZM7uRlrY6Vci1Z9i7gsZc2aOYtpnvBAyRSnJhOuKSDd1zR/zJ9zLcsmRmqn+gKGgW7deADbQt3NKAYMwAVzcgJ+mZWYuMtloJVHV04boKwhsCwLtYUQl7URzntx40rnXlM3pY+CIZiUVMROU0DmWAPLyuhLtgw99hABb04aRhYCw== 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 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. > >> + 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