linux-kernel - Re: [PATCH v1 04/12] mm/rmap: implement make_device_exclusive() using folio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fp54u2cgvyyp73lpp7bbc5zn7fmyoc7pjn3sokqmlvpq5y7toq@mj6dshe3hg7o>
Date: Fri, 31 Jan 2025 09:31:13 +1100
From: Alistair Popple <apopple@...dia.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, linux-mm@...ck.org, nouveau@...ts.freedesktop.org, 
	Andrew Morton <akpm@...ux-foundation.org>, Jérôme Glisse <jglisse@...hat.com>, 
	Jonathan Corbet <corbet@....net>, Alex Shi <alexs@...nel.org>, Yanteng Si <si.yanteng@...ux.dev>, 
	Karol Herbst <kherbst@...hat.com>, Lyude Paul <lyude@...hat.com>, 
	Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>, 
	Pasha Tatashin <pasha.tatashin@...een.com>, Peter Xu <peterx@...hat.com>, Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH v1 04/12] mm/rmap: implement make_device_exclusive()
 using folio_walk instead of rmap walk

On Thu, Jan 30, 2025 at 10:24:37AM +0100, David Hildenbrand wrote:
> On 30.01.25 10:01, David Hildenbrand wrote:
> > On 30.01.25 07:11, Alistair Popple wrote:
> > > On Wed, Jan 29, 2025 at 12:54:02PM +0100, David Hildenbrand wrote:
> > > > We require a writable PTE and only support anonymous folio: we can only
> > > > have exactly one PTE pointing at that page, which we can just lookup
> > > > using a folio walk, avoiding the rmap walk and the anon VMA lock.
> > > > 
> > > > So let's stop doing an rmap walk and perform a folio walk instead, so we
> > > > can easily just modify a single PTE and avoid relying on rmap/mapcounts.
> > > > 
> > > > We now effectively work on a single PTE instead of multiple PTEs of
> > > > a large folio, allowing for conversion of individual PTEs from
> > > > non-exclusive to device-exclusive -- note that the other way always
> > > > worked on single PTEs.
> > > > 
> > > > We can drop the MMU_NOTIFY_EXCLUSIVE MMU notifier call and document why
> > > > that is not required: GUP will already take care of the
> > > > MMU_NOTIFY_EXCLUSIVE call if required (there is already a device-exclusive
> > > > entry) when not finding a present PTE and having to trigger a fault and
> > > > ending up in remove_device_exclusive_entry().
> > > 
> > > I will have to look at this a bit more closely tomorrow but this doesn't seem
> > > right to me. We may be transitioning from a present PTE (ie. a writable
> > > anonymous mapping) to a non-present PTE (ie. a device-exclusive entry) and
> > > therefore any secondary processors (eg. other GPUs, iommus, etc.) will need to
> > > update their copies of the PTE. So I think the notifier call is needed.
> > 
> > Then it is all very confusing:

Can't argue with that in hindsight :-)

> > "MMU_NOTIFY_EXCLUSIVE: to signal a device driver that the device will no
> > longer have exclusive access to the page."
> 
> So the second sentence actually describes the other condition. Likely we
> should make that clearer:
>
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -43,10 +43,11 @@ struct mmu_interval_notifier;
>   * a device driver to possibly ignore the invalidation if the
>   * owner field matches the driver's device private pgmap owner.
>   *
> - * @MMU_NOTIFY_EXCLUSIVE: to signal a device driver that the device will no
> - * longer have exclusive access to the page. When sent during creation of an
> - * exclusive range the owner will be initialised to the value provided by the
> - * caller of make_device_exclusive(), otherwise the owner will be NULL.
> + * @MMU_NOTIFY_EXCLUSIVE: (1) to signal a device driver that the device will no
> + * longer have exclusive access to the page; and (2) to signal that a page will
> + * be made exclusive to a device. During (1), the owner will be NULL, during
> + * (2), the owner will be initialised to the value provided by the caller of
> + * make_device_exclusive().

Yes, I think that makes things clearer. Logically these are really two different
events though - I guess I didn't want to add another one at the time but I
wonder if we should just make them separate events rather than overloading them?

>   */
>  enum mmu_notifier_event {
>         MMU_NOTIFY_UNMAP = 0,
> 
> 
> -- 
> Cheers,
> 
> David / dhildenb
>