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
| ||
|
Message-ID: <74dcaab4-7857-c181-44cf-f1c273843520@amd.com> Date: Fri, 15 Jul 2022 09:12:59 -0500 From: "Sierra Guiza, Alejandro (Alex)" <alex.sierra@....com> To: Alistair Popple <apopple@...dia.com> Cc: Matthew Wilcox <willy@...radead.org>, jgg@...dia.com, Felix.Kuehling@....com, linux-mm@...ck.org, rcampbell@...dia.com, linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, hch@....de, jglisse@...hat.com, akpm@...ux-foundation.org Subject: Re: [PATCH] mm/gup: migrate device coherent pages when pinning instead of failing On 7/14/2022 9:11 PM, Alistair Popple wrote: > Currently any attempts to pin a device coherent page will fail. This is > because device coherent pages need to be managed by a device driver, and > pinning them would prevent a driver from migrating them off the device. > > However this is no reason to fail pinning of these pages. These are > coherent and accessible from the CPU so can be migrated just like > pinning ZONE_MOVABLE pages. So instead of failing all attempts to pin > them first try migrating them out of ZONE_DEVICE. > > [hch: rebased to the split device memory checks, > moved migrate_device_page to migrate_device.c] > > Signed-off-by: Alistair Popple <apopple@...dia.com> > Acked-by: Felix Kuehling <Felix.Kuehling@....com> > Signed-off-by: Christoph Hellwig <hch@....de> > --- > > This patch hopefully addresses all of David's comments. It replaces both my "mm: > remove the vma check in migrate_vma_setup()" and "mm/gup: migrate device > coherent pages when pinning instead of failing" patches. I'm not sure what the > best way of including this is, perhaps Alex can respin the series with this > patch instead? For sure Alistair. I'll include this in my next patch series version. Thanks, Alex Sierra > > - Alistair > > mm/gup.c | 50 +++++++++++++++++++++++++++++++++++++------ > mm/internal.h | 1 + > mm/migrate_device.c | 52 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 96 insertions(+), 7 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index b65fe8bf5af4..22b97ab61cd9 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1881,7 +1881,7 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, > unsigned long isolation_error_count = 0, i; > struct folio *prev_folio = NULL; > LIST_HEAD(movable_page_list); > - bool drain_allow = true; > + bool drain_allow = true, coherent_pages = false; > int ret = 0; > > for (i = 0; i < nr_pages; i++) { > @@ -1891,9 +1891,38 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, > continue; > prev_folio = folio; > > - if (folio_is_longterm_pinnable(folio)) > + /* > + * Device coherent pages are managed by a driver and should not > + * be pinned indefinitely as it prevents the driver moving the > + * page. So when trying to pin with FOLL_LONGTERM instead try > + * to migrate the page out of device memory. > + */ > + if (folio_is_device_coherent(folio)) { > + /* > + * We always want a new GUP lookup with device coherent > + * pages. > + */ > + pages[i] = 0; > + coherent_pages = true; > + > + /* > + * Migration will fail if the page is pinned, so convert > + * the pin on the source page to a normal reference. > + */ > + if (gup_flags & FOLL_PIN) { > + get_page(&folio->page); > + unpin_user_page(&folio->page); > + } > + > + ret = migrate_device_coherent_page(&folio->page); > + if (ret) > + goto unpin_pages; > + > continue; > + } > > + if (folio_is_longterm_pinnable(folio)) > + continue; > /* > * Try to move out any movable page before pinning the range. > */ > @@ -1919,7 +1948,8 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, > folio_nr_pages(folio)); > } > > - if (!list_empty(&movable_page_list) || isolation_error_count) > + if (!list_empty(&movable_page_list) || isolation_error_count > + || coherent_pages) > goto unpin_pages; > > /* > @@ -1929,10 +1959,16 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, > return nr_pages; > > unpin_pages: > - if (gup_flags & FOLL_PIN) { > - unpin_user_pages(pages, nr_pages); > - } else { > - for (i = 0; i < nr_pages; i++) > + /* > + * pages[i] might be NULL if any device coherent pages were found. > + */ > + for (i = 0; i < nr_pages; i++) { > + if (!pages[i]) > + continue; > + > + if (gup_flags & FOLL_PIN) > + unpin_user_page(pages[i]); > + else > put_page(pages[i]); > } > > diff --git a/mm/internal.h b/mm/internal.h > index c0f8fbe0445b..899dab512c5a 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -853,6 +853,7 @@ int numa_migrate_prep(struct page *page, struct vm_area_struct *vma, > unsigned long addr, int page_nid, int *flags); > > void free_zone_device_page(struct page *page); > +int migrate_device_coherent_page(struct page *page); > > /* > * mm/gup.c > diff --git a/mm/migrate_device.c b/mm/migrate_device.c > index 18bc6483f63a..7feeb447e3b9 100644 > --- a/mm/migrate_device.c > +++ b/mm/migrate_device.c > @@ -686,6 +686,12 @@ void migrate_vma_pages(struct migrate_vma *migrate) > } > > if (!page) { > + /* > + * The only time there is no vma is when called from > + * migrate_device_coherent_page(). However this isn't > + * called if the page could not be unmapped. > + */ > + VM_BUG_ON(!migrate->vma); > if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE)) > continue; > if (!notified) { > @@ -794,3 +800,49 @@ void migrate_vma_finalize(struct migrate_vma *migrate) > } > } > EXPORT_SYMBOL(migrate_vma_finalize); > + > +/* > + * Migrate a device coherent page back to normal memory. The caller should have > + * a reference on page which will be copied to the new page if migration is > + * successful or dropped on failure. > + */ > +int migrate_device_coherent_page(struct page *page) > +{ > + unsigned long src_pfn, dst_pfn = 0; > + struct migrate_vma args; > + struct page *dpage; > + > + WARN_ON_ONCE(PageCompound(page)); > + > + lock_page(page); > + src_pfn = migrate_pfn(page_to_pfn(page)) | MIGRATE_PFN_MIGRATE; > + args.src = &src_pfn; > + args.dst = &dst_pfn; > + args.cpages = 1; > + args.npages = 1; > + args.vma = NULL; > + > + /* > + * We don't have a VMA and don't need to walk the page tables to find > + * the source page. So call migrate_vma_unmap() directly to unmap the > + * page as migrate_vma_setup() will fail if args.vma == NULL. > + */ > + migrate_vma_unmap(&args); > + if (!(src_pfn & MIGRATE_PFN_MIGRATE)) > + return -EBUSY; > + > + dpage = alloc_page(GFP_USER | __GFP_NOWARN); > + if (dpage) { > + lock_page(dpage); > + dst_pfn = migrate_pfn(page_to_pfn(dpage)); > + } > + > + migrate_vma_pages(&args); > + if (src_pfn & MIGRATE_PFN_MIGRATE) > + copy_highpage(dpage, page); > + migrate_vma_finalize(&args); > + > + if (src_pfn & MIGRATE_PFN_MIGRATE) > + return 0; > + return -EBUSY; > +}
Powered by blists - more mailing lists