From: Matthew Wilcox <willy@infradead.org> To: Alistair Popple <apopple@nvidia.com> Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, Ralph Campbell <rcampbell@nvidia.com>, John Hubbard <jhubbard@nvidia.com>, nouveau@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [PATCH] mm: Take a page reference when removing device exclusive entries Date: Wed, 29 Mar 2023 04:16:25 +0100 [thread overview] Message-ID: <ZCOtiZFoxC6w/eoZ@casper.infradead.org> (raw) In-Reply-To: <20230328021434.292971-1-apopple@nvidia.com> On Tue, Mar 28, 2023 at 01:14:34PM +1100, Alistair Popple wrote: > +++ b/mm/memory.c > @@ -3623,8 +3623,19 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) > struct vm_area_struct *vma = vmf->vma; > struct mmu_notifier_range range; > > - if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) > + /* > + * We need a page reference to lock the page because we don't > + * hold the PTL so a racing thread can remove the > + * device-exclusive entry and unmap the page. If the page is > + * free the entry must have been removed already. > + */ > + if (!get_page_unless_zero(vmf->page)) > + return 0; From a folio point of view: what the hell are you doing here? Tail pages don't have individual refcounts; all the refcounts are actually taken on the folio. So this should be: if (!folio_try_get(folio)) return 0; (you can fix up the comment yourself) > + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { > + put_page(vmf->page); folio_put(folio); > return VM_FAULT_RETRY; > + } > mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma, > vma->vm_mm, vmf->address & PAGE_MASK, > (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); > @@ -3637,6 +3648,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) > > pte_unmap_unlock(vmf->pte, vmf->ptl); > folio_unlock(folio); > + put_page(vmf->page); folio_put(folio) There, I just saved you 3 calls to compound_head(), saving roughly 150 bytes of kernel text. > mmu_notifier_invalidate_range_end(&range); > return 0; > -- > 2.39.2 > >
WARNING: multiple messages have this Message-ID (diff)
From: Matthew Wilcox <willy@infradead.org> To: Alistair Popple <apopple@nvidia.com> Cc: Ralph Campbell <rcampbell@nvidia.com>, nouveau@lists.freedesktop.org, stable@vger.kernel.org, linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org> Subject: Re: [Nouveau] [PATCH] mm: Take a page reference when removing device exclusive entries Date: Wed, 29 Mar 2023 04:16:25 +0100 [thread overview] Message-ID: <ZCOtiZFoxC6w/eoZ@casper.infradead.org> (raw) In-Reply-To: <20230328021434.292971-1-apopple@nvidia.com> On Tue, Mar 28, 2023 at 01:14:34PM +1100, Alistair Popple wrote: > +++ b/mm/memory.c > @@ -3623,8 +3623,19 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) > struct vm_area_struct *vma = vmf->vma; > struct mmu_notifier_range range; > > - if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) > + /* > + * We need a page reference to lock the page because we don't > + * hold the PTL so a racing thread can remove the > + * device-exclusive entry and unmap the page. If the page is > + * free the entry must have been removed already. > + */ > + if (!get_page_unless_zero(vmf->page)) > + return 0; From a folio point of view: what the hell are you doing here? Tail pages don't have individual refcounts; all the refcounts are actually taken on the folio. So this should be: if (!folio_try_get(folio)) return 0; (you can fix up the comment yourself) > + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { > + put_page(vmf->page); folio_put(folio); > return VM_FAULT_RETRY; > + } > mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma, > vma->vm_mm, vmf->address & PAGE_MASK, > (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); > @@ -3637,6 +3648,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) > > pte_unmap_unlock(vmf->pte, vmf->ptl); > folio_unlock(folio); > + put_page(vmf->page); folio_put(folio) There, I just saved you 3 calls to compound_head(), saving roughly 150 bytes of kernel text. > mmu_notifier_invalidate_range_end(&range); > return 0; > -- > 2.39.2 > >
next prev parent reply other threads:[~2023-03-29 3:16 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-28 2:14 [PATCH] mm: Take a page reference when removing device exclusive entries Alistair Popple 2023-03-28 2:14 ` [Nouveau] " Alistair Popple 2023-03-28 6:25 ` John Hubbard 2023-03-28 6:25 ` [Nouveau] " John Hubbard 2023-03-28 19:55 ` Andrew Morton 2023-03-28 19:55 ` [Nouveau] " Andrew Morton 2023-03-29 1:45 ` Alistair Popple 2023-03-29 1:45 ` [Nouveau] " Alistair Popple 2023-03-28 19:59 ` Ralph Campbell 2023-03-28 19:59 ` Ralph Campbell 2023-03-29 3:16 ` Matthew Wilcox [this message] 2023-03-29 3:16 ` [Nouveau] " Matthew Wilcox 2023-03-29 3:45 ` John Hubbard 2023-03-29 3:45 ` [Nouveau] " John Hubbard 2023-03-30 0:26 ` Alistair Popple 2023-03-30 0:26 ` [Nouveau] " Alistair Popple 2023-03-30 0:55 ` Alistair Popple 2023-03-30 0:55 ` [Nouveau] " Alistair Popple
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZCOtiZFoxC6w/eoZ@casper.infradead.org \ --to=willy@infradead.org \ --cc=akpm@linux-foundation.org \ --cc=apopple@nvidia.com \ --cc=jhubbard@nvidia.com \ --cc=linux-mm@kvack.org \ --cc=nouveau@lists.freedesktop.org \ --cc=rcampbell@nvidia.com \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.