| Message ID | 9c21d7ed27117f6a2c2ef86fe9d2d88e4c8c8ad4.1712796818.git-series.apopple@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs/dax: Fix FS DAX page reference counts | expand |
On Thu, Apr 11, 2024 at 10:57:28AM +1000, Alistair Popple wrote: > Zone device pages are used to represent various type of device memory > managed by device drivers. Currently compound zone device pages are > not supported. This is because MEMORY_DEVICE_FS_DAX pages are the only > user of higher order zone device pages and have their own page > reference counting. > > A future change will unify FS DAX reference counting with normal page > reference counting rules and remove the special FS DAX reference > counting. Supporting that requires compound zone device pages. > > Supporting compound zone device pages requires compound_head() to > distinguish between head and tail pages whilst still preserving the > special struct page fields that are specific to zone device pages. > > A tail page is distinguished by having bit zero being set in > page->compound_head, with the remaining bits pointing to the head > page. For zone device pages page->compound_head is shared with > page->pgmap. > > The page->pgmap field is common to all pages within a memory section. > Therefore pgmap is the same for both head and tail pages and we can > use the same scheme to distinguish tail pages. To obtain the pgmap for > a tail page a new accessor is introduced to fetch it from > compound_head. > > Signed-off-by: Alistair Popple <apopple@nvidia.com> > --- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +- > drivers/pci/p2pdma.c | 2 +- > include/linux/memremap.h | 12 +++++++++--- > include/linux/migrate.h | 2 +- > lib/test_hmm.c | 2 +- > mm/hmm.c | 2 +- > mm/memory.c | 2 +- > mm/memremap.c | 6 +++--- > mm/migrate_device.c | 4 ++-- > 9 files changed, 20 insertions(+), 14 deletions(-) Makes sense to me Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On Thu, Apr 11, 2024 at 10:57:28AM +1000, Alistair Popple wrote: > Supporting compound zone device pages requires compound_head() to > distinguish between head and tail pages whilst still preserving the > special struct page fields that are specific to zone device pages. > > A tail page is distinguished by having bit zero being set in > page->compound_head, with the remaining bits pointing to the head > page. For zone device pages page->compound_head is shared with > page->pgmap. > > The page->pgmap field is common to all pages within a memory section. > Therefore pgmap is the same for both head and tail pages and we can > use the same scheme to distinguish tail pages. To obtain the pgmap for > a tail page a new accessor is introduced to fetch it from > compound_head. Would it make sense at this point to move pgmap and zone_device_data from struct page to struct folio? That will make any forgotten places fail to compile instead of getting a bogus value.
Matthew Wilcox <willy@infradead.org> writes: > On Thu, Apr 11, 2024 at 10:57:28AM +1000, Alistair Popple wrote: >> Supporting compound zone device pages requires compound_head() to >> distinguish between head and tail pages whilst still preserving the >> special struct page fields that are specific to zone device pages. >> >> A tail page is distinguished by having bit zero being set in >> page->compound_head, with the remaining bits pointing to the head >> page. For zone device pages page->compound_head is shared with >> page->pgmap. >> >> The page->pgmap field is common to all pages within a memory section. >> Therefore pgmap is the same for both head and tail pages and we can >> use the same scheme to distinguish tail pages. To obtain the pgmap for >> a tail page a new accessor is introduced to fetch it from >> compound_head. > > Would it make sense at this point to move pgmap and zone_device_data > from struct page to struct folio? That will make any forgotten > places fail to compile instead of getting a bogus value. Thanks for the suggestion, I think that makes a lot of sense. Will try it for the next version to see what it looks like.
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c index 12feecf..eb49f07 100644 --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c @@ -88,7 +88,7 @@ struct nouveau_dmem { static struct nouveau_dmem_chunk *nouveau_page_to_chunk(struct page *page) { - return container_of(page->pgmap, struct nouveau_dmem_chunk, pagemap); + return container_of(page_dev_pagemap(page), struct nouveau_dmem_chunk, pagemap); } static struct nouveau_drm *page_to_drm(struct page *page) diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index ab7ef18..dfc2a17 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -195,7 +195,7 @@ static const struct attribute_group p2pmem_group = { static void p2pdma_page_free(struct page *page) { - struct pci_p2pdma_pagemap *pgmap = to_p2p_pgmap(page->pgmap); + struct pci_p2pdma_pagemap *pgmap = to_p2p_pgmap(page_dev_pagemap(page)); /* safe to dereference while a reference is held to the percpu ref */ struct pci_p2pdma *p2pdma = rcu_dereference_protected(pgmap->provider->p2pdma, 1); diff --git a/include/linux/memremap.h b/include/linux/memremap.h index 1314d9c..0773f8b 100644 --- a/include/linux/memremap.h +++ b/include/linux/memremap.h @@ -139,6 +139,12 @@ struct dev_pagemap { }; }; +static inline struct dev_pagemap *page_dev_pagemap(const struct page *page) +{ + WARN_ON(!is_zone_device_page(page)); + return compound_head(page)->pgmap; +} + static inline bool pgmap_has_memory_failure(struct dev_pagemap *pgmap) { return pgmap->ops && pgmap->ops->memory_failure; @@ -160,7 +166,7 @@ static inline bool is_device_private_page(const struct page *page) { return IS_ENABLED(CONFIG_DEVICE_PRIVATE) && is_zone_device_page(page) && - page->pgmap->type == MEMORY_DEVICE_PRIVATE; + page_dev_pagemap(page)->type == MEMORY_DEVICE_PRIVATE; } static inline bool folio_is_device_private(const struct folio *folio) @@ -172,13 +178,13 @@ static inline bool is_pci_p2pdma_page(const struct page *page) { return IS_ENABLED(CONFIG_PCI_P2PDMA) && is_zone_device_page(page) && - page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; + page_dev_pagemap(page)->type == MEMORY_DEVICE_PCI_P2PDMA; } static inline bool is_device_coherent_page(const struct page *page) { return is_zone_device_page(page) && - page->pgmap->type == MEMORY_DEVICE_COHERENT; + page_dev_pagemap(page)->type == MEMORY_DEVICE_COHERENT; } static inline bool folio_is_device_coherent(const struct folio *folio) diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 711dd94..ebaf279 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -200,7 +200,7 @@ struct migrate_vma { unsigned long end; /* - * Set to the owner value also stored in page->pgmap->owner for + * Set to the owner value also stored in page_dev_pagemap(page)->owner for * migrating out of device private memory. The flags also need to * be set to MIGRATE_VMA_SELECT_DEVICE_PRIVATE. * The caller should always set this field when using mmu notifier diff --git a/lib/test_hmm.c b/lib/test_hmm.c index 717dcb8..1101ff4 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -195,7 +195,7 @@ static int dmirror_fops_release(struct inode *inode, struct file *filp) static struct dmirror_chunk *dmirror_page_to_chunk(struct page *page) { - return container_of(page->pgmap, struct dmirror_chunk, pagemap); + return container_of(page_dev_pagemap(page), struct dmirror_chunk, pagemap); } static struct dmirror_device *dmirror_page_to_device(struct page *page) diff --git a/mm/hmm.c b/mm/hmm.c index 5bbfb0e..a665a3c 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -248,7 +248,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, * just report the PFN. */ if (is_device_private_entry(entry) && - pfn_swap_entry_to_page(entry)->pgmap->owner == + page_dev_pagemap(pfn_swap_entry_to_page(entry))->owner == range->dev_private_owner) { cpu_flags = HMM_PFN_VALID; if (is_writable_device_private_entry(entry)) diff --git a/mm/memory.c b/mm/memory.c index 517221f..52248d4 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3768,7 +3768,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) */ get_page(vmf->page); pte_unmap_unlock(vmf->pte, vmf->ptl); - ret = vmf->page->pgmap->ops->migrate_to_ram(vmf); + ret = page_dev_pagemap(vmf->page)->ops->migrate_to_ram(vmf); put_page(vmf->page); } else if (is_hwpoison_entry(entry)) { ret = VM_FAULT_HWPOISON; diff --git a/mm/memremap.c b/mm/memremap.c index 99d26ff..619b059 100644 --- a/mm/memremap.c +++ b/mm/memremap.c @@ -470,7 +470,7 @@ EXPORT_SYMBOL_GPL(get_dev_pagemap); void free_zone_device_page(struct page *page) { - if (WARN_ON_ONCE(!page->pgmap->ops || !page->pgmap->ops->page_free)) + if (WARN_ON_ONCE(!page_dev_pagemap(page)->ops || !page_dev_pagemap(page)->ops->page_free)) return; mem_cgroup_uncharge(page_folio(page)); @@ -506,7 +506,7 @@ void free_zone_device_page(struct page *page) * to clear page->mapping. */ page->mapping = NULL; - page->pgmap->ops->page_free(page); + page_dev_pagemap(page)->ops->page_free(page); if (page->pgmap->type == MEMORY_DEVICE_PRIVATE || page->pgmap->type == MEMORY_DEVICE_COHERENT) @@ -525,7 +525,7 @@ void zone_device_page_init(struct page *page) * Drivers shouldn't be allocating pages after calling * memunmap_pages(). */ - WARN_ON_ONCE(!percpu_ref_tryget_live(&page->pgmap->ref)); + WARN_ON_ONCE(!percpu_ref_tryget_live(&page_dev_pagemap(page)->ref)); set_page_count(page, 1); lock_page(page); } diff --git a/mm/migrate_device.c b/mm/migrate_device.c index 8ac1f79..1e1c82f 100644 --- a/mm/migrate_device.c +++ b/mm/migrate_device.c @@ -134,7 +134,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, page = pfn_swap_entry_to_page(entry); if (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_PRIVATE) || - page->pgmap->owner != migrate->pgmap_owner) + page_dev_pagemap(page)->owner != migrate->pgmap_owner) goto next; mpfn = migrate_pfn(page_to_pfn(page)) | @@ -155,7 +155,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, goto next; else if (page && is_device_coherent_page(page) && (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_COHERENT) || - page->pgmap->owner != migrate->pgmap_owner)) + page_dev_pagemap(page)->owner != migrate->pgmap_owner)) goto next; mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE; mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
Zone device pages are used to represent various type of device memory managed by device drivers. Currently compound zone device pages are not supported. This is because MEMORY_DEVICE_FS_DAX pages are the only user of higher order zone device pages and have their own page reference counting. A future change will unify FS DAX reference counting with normal page reference counting rules and remove the special FS DAX reference counting. Supporting that requires compound zone device pages. Supporting compound zone device pages requires compound_head() to distinguish between head and tail pages whilst still preserving the special struct page fields that are specific to zone device pages. A tail page is distinguished by having bit zero being set in page->compound_head, with the remaining bits pointing to the head page. For zone device pages page->compound_head is shared with page->pgmap. The page->pgmap field is common to all pages within a memory section. Therefore pgmap is the same for both head and tail pages and we can use the same scheme to distinguish tail pages. To obtain the pgmap for a tail page a new accessor is introduced to fetch it from compound_head. Signed-off-by: Alistair Popple <apopple@nvidia.com> --- drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +- drivers/pci/p2pdma.c | 2 +- include/linux/memremap.h | 12 +++++++++--- include/linux/migrate.h | 2 +- lib/test_hmm.c | 2 +- mm/hmm.c | 2 +- mm/memory.c | 2 +- mm/memremap.c | 6 +++--- mm/migrate_device.c | 4 ++-- 9 files changed, 20 insertions(+), 14 deletions(-)