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 A619DC46467 for ; Wed, 18 Jan 2023 22:29:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2337D6B0071; Wed, 18 Jan 2023 17:29:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1E3296B0073; Wed, 18 Jan 2023 17:29:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 084366B0074; Wed, 18 Jan 2023 17:29:12 -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 EDA7E6B0071 for ; Wed, 18 Jan 2023 17:29:11 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id C90ED1C610A for ; Wed, 18 Jan 2023 22:29:11 +0000 (UTC) X-FDA: 80369361702.17.44DB22C Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf30.hostedemail.com (Postfix) with ESMTP id CC7F180010 for ; Wed, 18 Jan 2023 22:29:09 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ZQGAIUwS; spf=pass (imf30.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1674080949; 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=xZdbxU7U6Cq2n8pjRxmWipD5AezhqaGVZdCMLeOSIq0=; b=a349WWVVOnSepnNp824jlMOUNJAxbWtPBuqGyoGZyERm9zB05JlOcNYKrn+JQDfQpfHuLu Hwp91rN6DFcBoP3re3s4wrb4d02zCPmxMQETrjDZDRDraJEAJJf4xFL6HZJGUqtTE9dfw4 /YlipgA+eIkpELomAIjGapoFTz1I3hs= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ZQGAIUwS; spf=pass (imf30.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1674080950; a=rsa-sha256; cv=none; b=rHBRZzW0g6Z3gpCeKXHVfw2yK/2RcDKF0A8gBN9TRSBVHlIfDmNHWRPBWdU0AMq+CSVnc3 7gumDYyPCbWLai423qV/NKaUrVlvna6jYEhrcD2GtWmA6ttnbU33qa1FRsEPzt4BY2NDMb Ala0fo6mZFNf77jK9s+X/QFDZTdVBvA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674080949; 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: in-reply-to:in-reply-to:references:references; bh=xZdbxU7U6Cq2n8pjRxmWipD5AezhqaGVZdCMLeOSIq0=; b=ZQGAIUwSoqLRrqCz9dhR5B83vtbArjcomHwgzg7eRBe/tI6BVctSiaUH/THLGCctR4iTTn Nwog5O10TnyeT6bimheOUGDE83jBgqXSy82J+r4SkvUZX2umE7zZHyRpxOQrKauap8O7vJ rdTO7MR6Am6e8KIULC09SE9lZXbNrwo= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-455-2--fIY48PrCwZnA8f7VRpw-1; Wed, 18 Jan 2023 17:29:03 -0500 X-MC-Unique: 2--fIY48PrCwZnA8f7VRpw-1 Received: by mail-qk1-f198.google.com with SMTP id j10-20020a05620a288a00b0070630ecfd9bso303739qkp.20 for ; Wed, 18 Jan 2023 14:29:03 -0800 (PST) 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=xZdbxU7U6Cq2n8pjRxmWipD5AezhqaGVZdCMLeOSIq0=; b=fACzeGKpKQzWP0snnTqF0bOY/ZXstKoloTPeg5Jy8zWKz/KqLcfULWvIINjZB43xBP rwgijeX6EkIxD+Hmqqx1DqfhspoTaqMj3ht8FByJuv/NC61sKKfCWbA7ZyLjkaD/LmCs 8wfnlINg7pEgpmwEHnPrL4zUhYzkgnihrRQfQbfad0Dqn70okeNfEUcHrnJ1/+J6Xevr MB0zFuz6b+UBImVax1Cpmml6RyGhSsSFrf25aJifz0/5RMlc77TGkJEGXX6VHk0QYGJv EbBMtXg4FRT/gb5qEA8d2wFWRgZCOBG5DlCcZo8DV3KAwSNtzAdVREyYO7dE6kLnluMb tGtw== X-Gm-Message-State: AFqh2kq9l5mhcNz5Wt7hKIkYjPJ7JsCVfv9mh9yCVa/KVsSFLUIbBSM8 eWAYgEbXfTHrKpVVTDbZGNyS6orEdc1I0/pJqTXcU/gilprQdK2m/StNCfa5FX3AQx2L/j5hR9+ RybSn/AWUjpg= X-Received: by 2002:a05:622a:6027:b0:3a5:264c:5f38 with SMTP id he39-20020a05622a602700b003a5264c5f38mr41792373qtb.63.1674080942321; Wed, 18 Jan 2023 14:29:02 -0800 (PST) X-Google-Smtp-Source: AMrXdXsl9OIsfPXCScNAje7vE8rqb3VJvdYvO/ETOWbzggmAZM3fZJJf/SErl3b5gRWagnicf2gKWw== X-Received: by 2002:a05:622a:6027:b0:3a5:264c:5f38 with SMTP id he39-20020a05622a602700b003a5264c5f38mr41792335qtb.63.1674080941983; Wed, 18 Jan 2023 14:29:01 -0800 (PST) Received: from x1n (bras-base-aurron9127w-grc-56-70-30-145-63.dsl.bell.ca. [70.30.145.63]) by smtp.gmail.com with ESMTPSA id j9-20020ac806c9000000b003ab5d6cd6c5sm18027439qth.16.2023.01.18.14.28.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Jan 2023 14:29:01 -0800 (PST) Date: Wed, 18 Jan 2023 17:28:58 -0500 From: Peter Xu To: Muhammad Usama Anjum Cc: David Hildenbrand , Andrew Morton , =?utf-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= , Andrei Vagin , Danylo Mocherniuk , Paul Gofman , Cyrill Gorcunov , Alexander Viro , Shuah Khan , Christian Brauner , Yang Shi , Vlastimil Babka , "Liam R . Howlett" , Yun Zhou , Suren Baghdasaryan , Alex Sierra , Matthew Wilcox , Pasha Tatashin , Mike Rapoport , Nadav Amit , 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 v7 3/4] fs/proc/task_mmu: Implement IOCTL to get and/or the clear info about PTEs Message-ID: References: <20230109064519.3555250-1-usama.anjum@collabora.com> <20230109064519.3555250-4-usama.anjum@collabora.com> MIME-Version: 1.0 In-Reply-To: <20230109064519.3555250-4-usama.anjum@collabora.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: u3chor6ja17fjgq1x1zyiwpjttd7cxjm X-Rspamd-Queue-Id: CC7F180010 X-HE-Tag: 1674080949-718696 X-HE-Meta: U2FsdGVkX19qhIO8WmQvKGZH5hJRZVnBCl0iD5AdHbhyI1/YQcl7tU1+7I4QPAUKAnyVWzYf8GkGlcqtJS+PGbMXG3bWcHhYaaNX2pO/fLEGA5Sw/nGzdumX3RfpDKYx7rsKholYS8WLv9+mShkQpqIvplmmeOQ1TxqK0Qjp5JVARboExDDVhu22qmz1oNpdOTtUhlh4xmcoIQHZKz0sxYSEkOqU/TCwzBW/t4lzrGxUMXknqWVXmye0D+YY/bLL1EC0/v48zatkI4XobwPYgl4ccmAp2mq/I6lkOQzM+IsAao7wvy2ZbWt26uhvVZs0BiFE8SmXeiMpYleFvRepKMxWtG9E9oM4tO/+u9rB/R2UEBbQaUEV5SpvTZ7t/EQBXXXxrsjCa+bFsxWI3lT5dgpJ0HVrmtKkXTMFKgo+6Oq8Beoevawp+9d74+NJW/yguMQffaY8+L3J+fm1qPpOorYrs+1lbHkpQN/A1CFN0HMf/EqMsO0E5+wSr2ej3Dl+itwE9hNHwQGy+RRaU5py6WJu8PmbFUNZLDvfsc/Odlj3nbvdc+T5oYfCxObrpDgrmWmktz35QVDRJjqTcbeJCkjAGI+VWCSXDbjSL8RG3zLe02dWIEtjb/eH+CNeQ0cw8fsaU7jo45MqAunN6Krs8fJRGDBaTJsEcgWhnlWODV3OQeWNymq57nTG3oBMyr3GZK2p8kil5yLakBARtU67ANR3XJDjsEWCtTsbMLRm0gydikHSS1yRgi3qDcZ5R+LBw+VaqzSMMLwkS1usLep3A+gXted7kMlfBi+SmQOVL1fll88zRQunQVW9MqgdTKEzGbTXmajBWPTfTjnc39XxSuaYMSmnoba7CaaokV41V3HvNa/l2e4b2/lwaaice886U0w1rhfSS4mN0aeKbMyOvnbxmyn9RkrHgt/H5Yp3qpf9I7llWb18TGNGmjeLCjxgmi/7JEoRWArPpRCjf+h 07CiaWOT nvq5nzQcjiGq2xrePfbdxEO0uSjOPR91N/tZFLRgSfrVq+X2do9L96vFS8P0mAOcBOWtn39JsK4iHFoUNshsfh8fvz9q2cVTq9ax6rr7fuo+TWOupgDi9jOT+P6GyHdsx4JWD91BEU7hVNV8ZEvL1HlZkeP+NjxW7nDRR 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 09, 2023 at 11:45:18AM +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_Wt), > file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped > (PAGE_IS_SWAPPED). > - Write-protect the pages (PAGEMAP_WP_ENGAGE) to start finding which > pages have been written-to. > - Find pages which have been written-to and write protect the pages > (atomic PAGE_IS_NOT_WP + PAGEMAP_WP_ENGAGE) > > The uffd should have been registered on the memory range before performing > any WP/WT (Write Protect/Writtern-To) related operations with the IOCTL. > > struct pagemap_scan_args is used as the argument of the IOCTL. In this > struct: > - The range is specified through start and len. > - The output buffer and size is specified as vec and vec_len. > - The optional maximum requested pages are specified in the max_pages. > - The flags can be specified in the flags field. The PAGEMAP_WP_ENGAGE > is the only added flag at this time. > - The masks are specified in required_mask, anyof_mask, excluded_ mask > and return_mask. > > This IOCTL can be extended to get information about more PTE bits. This > patch has evolved from a basic patch from Gabriel Krisman Bertazi. > > Signed-off-by: Muhammad Usama Anjum > --- > Changes in v7: > - Rebase on top of latest next > - Fix some corner cases > - Base soft-dirty on the uffd wp async > - Update the terminologies > - Optimize the memory usage inside the ioctl > > Changes in v6: > - Rename variables and update comments > - Make IOCTL independent of soft_dirty config > - Change masks and bitmap type to _u64 > - Improve code quality > > Changes in v5: > - Remove tlb flushing even for clear operation > > Changes in v4: > - Update the interface and implementation > > Changes in v3: > - Tighten the user-kernel interface by using explicit types and add more > error checking > > Changes in v2: > - Convert the interface from syscall to ioctl > - Remove pidfd support as it doesn't make sense in ioctl > --- > fs/proc/task_mmu.c | 300 ++++++++++++++++++++++++++++++++++ > include/uapi/linux/fs.h | 50 ++++++ > tools/include/uapi/linux/fs.h | 50 ++++++ > 3 files changed, 400 insertions(+) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index e35a0398db63..ba70faadf403 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1135,6 +1136,22 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma, > } > #endif > > +static inline bool is_pte_uffd_wp(pte_t pte) > +{ > + if ((pte_present(pte) && pte_uffd_wp(pte)) || > + (is_swap_pte(pte) && pte_swp_uffd_wp(pte))) > + return true; This is an interesting way to detect whether the page is written.. I would have thought you would reuse soft-dirty bit. For swap case, you missed the pte markers (which can exist for !anon memory). You can have a look at pte_swp_uffd_wp_any(). > + return false; > +} > + > +static inline bool is_pmd_uffd_wp(pmd_t pmd) > +{ > + if ((pmd_present(pmd) && pmd_uffd_wp(pmd)) || > + (is_swap_pmd(pmd) && pmd_swp_uffd_wp(pmd))) > + return true; > + return false; > +} > + > #if defined(CONFIG_MEM_SOFT_DIRTY) && defined(CONFIG_TRANSPARENT_HUGEPAGE) > static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma, > unsigned long addr, pmd_t *pmdp) > @@ -1763,11 +1780,294 @@ static int pagemap_release(struct inode *inode, struct file *file) > return 0; > } > > +#define PAGEMAP_OP_MASK (PAGE_IS_WT | PAGE_IS_FILE | \ > + PAGE_IS_PRESENT | PAGE_IS_SWAPPED) > +#define PAGEMAP_NONWT_OP_MASK (PAGE_IS_FILE | PAGE_IS_PRESENT | PAGE_IS_SWAPPED) > +#define IS_WP_ENGAGE_OP(a) (a->flags & PAGEMAP_WP_ENGAGE) > +#define IS_GET_OP(a) (a->vec) > +#define PAGEMAP_SCAN_BITMAP(wt, file, present, swap) \ > + (wt | file << 1 | present << 2 | swap << 3) > +#define IS_WT_REQUIRED(a) \ > + ((a->required_mask & PAGE_IS_WT) || \ > + (a->anyof_mask & PAGE_IS_WT)) > + > +struct pagemap_scan_private { > + struct page_region *vec; > + struct page_region prev; > + unsigned long vec_len, vec_index; > + unsigned int max_pages, found_pages, flags; > + unsigned long required_mask, anyof_mask, excluded_mask, return_mask; > +}; > + > +static int pagemap_scan_test_walk(unsigned long start, unsigned long end, struct mm_walk *walk) > +{ > + struct pagemap_scan_private *p = walk->private; > + struct vm_area_struct *vma = walk->vma; > + > + if (IS_WT_REQUIRED(p) && !vma_can_userfault(vma, vma->vm_flags)) > + return -EPERM; vma_can_userfault() is too coarse. Maybe what you wanted to check is userfaultfd_wp(vma)? > + if (IS_GET_OP(p) && p->max_pages && (p->found_pages == p->max_pages)) > + return -ENOSPC; This is the function to test "whether the walker should walk the vma specified". This check should IIUC be meaningless because found_pages doesn't boost during vma switching, while OTOH your pmd walker fn should do proper check when increasing found_pages and return -ENOSPC properly when the same condition met. That should be enough, IMHO. I saw it already a few times in this patch, I think maybe you want a helper for this one taking *p as argument. > + if (vma->vm_flags & VM_PFNMAP) > + return 1; > + return 0; > +} > + > +static inline int add_to_out(bool wt, bool file, bool pres, bool swap, > + struct pagemap_scan_private *p, unsigned long addr, unsigned int len) > +{ > + unsigned long bitmap, cur = PAGEMAP_SCAN_BITMAP(wt, file, pres, swap); > + bool cpy = true; > + struct page_region *prev = &p->prev; > + > + if (p->required_mask) > + cpy = ((p->required_mask & cur) == p->required_mask); > + if (cpy && p->anyof_mask) > + cpy = (p->anyof_mask & cur); > + if (cpy && p->excluded_mask) > + cpy = !(p->excluded_mask & cur); > + bitmap = cur & p->return_mask; > + if (cpy && bitmap) { > + if ((prev->len) && (prev->bitmap == bitmap) && > + (prev->start + prev->len * PAGE_SIZE == addr)) { > + prev->len += len; > + p->found_pages += len; > + } else if (p->vec_index < p->vec_len) { > + if (prev->len) { > + memcpy(&p->vec[p->vec_index], prev, sizeof(struct page_region)); > + p->vec_index++; > + } > + prev->start = addr; > + prev->len = len; > + prev->bitmap = bitmap; > + p->found_pages += len; > + } else { > + return -ENOSPC; > + } [1] > + } > + return 0; > +} > + > +static inline int export_prev_to_out(struct pagemap_scan_private *p, struct page_region __user *vec, > + unsigned long *vec_index) > +{ > + struct page_region *prev = &p->prev; > + > + if (prev->len) { > + if (copy_to_user(&vec[*vec_index], prev, sizeof(struct page_region))) > + return -EFAULT; > + p->vec_index++; > + (*vec_index)++; > + prev->len = 0; > + } > + return 0; > +} I'd rather not have this helper; it doesn't really help a lot. > + > +static inline int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long addr, > + unsigned long end, struct mm_walk *walk) > +{ > + struct pagemap_scan_private *p = walk->private; > + struct vm_area_struct *vma = walk->vma; > + unsigned long start = addr; > + unsigned int len; > + spinlock_t *ptl; > + int ret = 0; > + pte_t *pte; > + bool pmd_wt; > + > + if ((walk->vma->vm_end < addr) || (p->max_pages && p->found_pages == p->max_pages)) > + return 0; > + end = min(end, walk->vma->vm_end); None of the check above is needed, I think.. vma ranges are checked before pmd_entry() called. found_pages should be checked when increased (I think it's [1] above). > + > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > + ptl = pmd_trans_huge_lock(pmd, vma); > + if (ptl) { > + pmd_wt = !is_pmd_uffd_wp(*pmd); > + /* > + * Break huge page into small pages if operation needs to be performed is > + * on a portion of the huge page or the return buffer cannot store complete > + * data. > + */ > + if (pmd_wt && (IS_WP_ENGAGE_OP(p) && (end - addr < HPAGE_SIZE))) { > + spin_unlock(ptl); > + split_huge_pmd(vma, pmd, addr); > + goto process_smaller_pages; > + } > + if (IS_GET_OP(p)) { > + len = (end - addr)/PAGE_SIZE; > + if (p->max_pages && p->found_pages + len > p->max_pages) > + len = p->max_pages - p->found_pages; > + > + ret = add_to_out(pmd_wt, vma->vm_file, pmd_present(*pmd), > + is_swap_pmd(*pmd), p, addr, len); > + if (ret) { > + spin_unlock(ptl); > + return ret; > + } > + } > + spin_unlock(ptl); > + if (IS_WP_ENGAGE_OP(p) && pmd_wt) { > + BUG_ON(start & ~HPAGE_MASK); > + > + ret = wp_range_async(vma, addr, HPAGE_SIZE); > + } > + return ret; > + } > +process_smaller_pages: > + if (pmd_trans_unstable(pmd)) > + return 0; > +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > + > + pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl); > + for (; addr < end && !ret && (!p->max_pages || (p->found_pages < p->max_pages)) > + ; pte++, addr += PAGE_SIZE) { > + if (IS_GET_OP(p)) { This 'if' should be out - skip loop if not needed. > + ret = add_to_out(!is_pte_uffd_wp(*pte), vma->vm_file, pte_present(*pte), > + is_swap_pte(*pte), p, addr, 1); > + if (ret) > + break; > + } > + } > + pte_unmap_unlock(pte - 1, ptl); > + if (IS_WP_ENGAGE_OP(p)) { > + BUG_ON(start & ~PAGE_MASK); > + ret = wp_range_async(vma, start, addr - start); > + } > + > + cond_resched(); > + return ret; > +} > + > +static int pagemap_scan_pte_hole(unsigned long addr, unsigned long end, int depth, > + struct mm_walk *walk) > +{ > + struct pagemap_scan_private *p = walk->private; > + struct vm_area_struct *vma = walk->vma; > + unsigned int len; > + int ret = 0; > + > + if (vma) { > + len = (end - addr)/PAGE_SIZE; > + if (p->max_pages && p->found_pages + len > p->max_pages) > + len = p->max_pages - p->found_pages; > + ret = add_to_out(false, vma->vm_file, false, false, p, addr, len); > + } > + > + return ret; > +} > + > +static const struct mm_walk_ops pagemap_scan_ops = { > + .test_walk = pagemap_scan_test_walk, > + .pmd_entry = pagemap_scan_pmd_entry, Do we care about hugetlb at all? So far it seems you don't. It's fine, then if you decided to go the uffd-wp way you can explicit declare no support on hugetlb, as uffd-wp sync supports it so it should be by default supported otherwise. > + .pte_hole = pagemap_scan_pte_hole, > +}; -- Peter Xu