| Message ID | 20220207063249.1833066-7-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/8] mm: remove a pointless CONFIG_ZONE_DEVICE check in memremap_pages | expand |
On 2022-02-06 11:32 p.m., Christoph Hellwig wrote: > Move the check for the actual pgmap types that need the free at refcount > one behavior into the out of line helper, and thus avoid the need to > pull memremap.h into mm.h. > > Signed-off-by: Christoph Hellwig <hch@lst.de> I've noticed mm/memcontrol.c uses is_device_private_page() and also needs a memremap.h include added to compile with my configuration. Logan
On Mon, Feb 07, 2022 at 07:32:47AM +0100, Christoph Hellwig wrote: > Move the check for the actual pgmap types that need the free at refcount > one behavior into the out of line helper, and thus avoid the need to > pull memremap.h into mm.h. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/arm64/mm/mmu.c | 1 + > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 + > drivers/gpu/drm/drm_cache.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + > drivers/gpu/drm/nouveau/nouveau_svm.c | 1 + > drivers/infiniband/core/rw.c | 1 + > drivers/nvdimm/pmem.h | 1 + > drivers/nvme/host/pci.c | 1 + > drivers/nvme/target/io-cmd-bdev.c | 1 + > fs/fuse/virtio_fs.c | 1 + > include/linux/memremap.h | 18 ++++++++++++++++++ > include/linux/mm.h | 20 -------------------- > lib/test_hmm.c | 1 + > mm/memremap.c | 6 +++++- > 14 files changed, 34 insertions(+), 22 deletions(-) Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
Am 2022-02-07 um 01:32 schrieb Christoph Hellwig: > Move the check for the actual pgmap types that need the free at refcount > one behavior into the out of line helper, and thus avoid the need to > pull memremap.h into mm.h. > > Signed-off-by: Christoph Hellwig <hch@lst.de> The amdkfd part looks good to me. It looks like this patch is not based on Alex Sierra's coherent memory series. He added two new helpers is_device_coherent_page and is_dev_private_or_coherent_page that would need to be moved along with is_device_private_page and is_pci_p2pdma_page. Acked-by: Felix Kuehling <Felix.Kuehling@amd.com> > --- > arch/arm64/mm/mmu.c | 1 + > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 + > drivers/gpu/drm/drm_cache.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + > drivers/gpu/drm/nouveau/nouveau_svm.c | 1 + > drivers/infiniband/core/rw.c | 1 + > drivers/nvdimm/pmem.h | 1 + > drivers/nvme/host/pci.c | 1 + > drivers/nvme/target/io-cmd-bdev.c | 1 + > fs/fuse/virtio_fs.c | 1 + > include/linux/memremap.h | 18 ++++++++++++++++++ > include/linux/mm.h | 20 -------------------- > lib/test_hmm.c | 1 + > mm/memremap.c | 6 +++++- > 14 files changed, 34 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index acfae9b41cc8c9..580abae6c0b93f 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -17,6 +17,7 @@ > #include <linux/mman.h> > #include <linux/nodemask.h> > #include <linux/memblock.h> > +#include <linux/memremap.h> > #include <linux/memory.h> > #include <linux/fs.h> > #include <linux/io.h> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index ea68f3b3a4e9cb..6d643b4b791d87 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -25,6 +25,7 @@ > > #include <linux/hashtable.h> > #include <linux/mmu_notifier.h> > +#include <linux/memremap.h> > #include <linux/mutex.h> > #include <linux/types.h> > #include <linux/atomic.h> > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index f19d9acbe95936..50b8a088f763a6 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -27,11 +27,11 @@ > /* > * Authors: Thomas Hellström <thomas-at-tungstengraphics-dot-com> > */ > - > #include <linux/dma-buf-map.h> > #include <linux/export.h> > #include <linux/highmem.h> > #include <linux/cc_platform.h> > +#include <linux/ioport.h> > #include <xen/xen.h> > > #include <drm/drm_cache.h> > diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c > index e886a3b9e08c7d..a5cdfbe32b5e54 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c > @@ -39,6 +39,7 @@ > > #include <linux/sched/mm.h> > #include <linux/hmm.h> > +#include <linux/memremap.h> > #include <linux/migrate.h> > > /* > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c > index 266809e511e2c1..090b9b47708cca 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c > @@ -35,6 +35,7 @@ > #include <linux/sched/mm.h> > #include <linux/sort.h> > #include <linux/hmm.h> > +#include <linux/memremap.h> > #include <linux/rmap.h> > > struct nouveau_svm { > diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c > index 5a3bd41b331c93..4d98f931a13ddd 100644 > --- a/drivers/infiniband/core/rw.c > +++ b/drivers/infiniband/core/rw.c > @@ -2,6 +2,7 @@ > /* > * Copyright (c) 2016 HGST, a Western Digital Company. > */ > +#include <linux/memremap.h> > #include <linux/moduleparam.h> > #include <linux/slab.h> > #include <linux/pci-p2pdma.h> > diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h > index 59cfe13ea8a85c..1f51a23614299b 100644 > --- a/drivers/nvdimm/pmem.h > +++ b/drivers/nvdimm/pmem.h > @@ -3,6 +3,7 @@ > #define __NVDIMM_PMEM_H__ > #include <linux/page-flags.h> > #include <linux/badblocks.h> > +#include <linux/memremap.h> > #include <linux/types.h> > #include <linux/pfn_t.h> > #include <linux/fs.h> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 6a99ed68091589..ab15bc72710dbe 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -15,6 +15,7 @@ > #include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/memremap.h> > #include <linux/mm.h> > #include <linux/module.h> > #include <linux/mutex.h> > diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c > index 70ca9dfc1771a9..a141446db1bea3 100644 > --- a/drivers/nvme/target/io-cmd-bdev.c > +++ b/drivers/nvme/target/io-cmd-bdev.c > @@ -6,6 +6,7 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > #include <linux/blkdev.h> > #include <linux/blk-integrity.h> > +#include <linux/memremap.h> > #include <linux/module.h> > #include "nvmet.h" > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 9d737904d07c0b..86b7dbb6a0d43e 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -8,6 +8,7 @@ > #include <linux/dax.h> > #include <linux/pci.h> > #include <linux/pfn_t.h> > +#include <linux/memremap.h> > #include <linux/module.h> > #include <linux/virtio.h> > #include <linux/virtio_fs.h> > diff --git a/include/linux/memremap.h b/include/linux/memremap.h > index 1fafcc38acbad6..514ab46f597e5c 100644 > --- a/include/linux/memremap.h > +++ b/include/linux/memremap.h > @@ -1,6 +1,8 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > #ifndef _LINUX_MEMREMAP_H_ > #define _LINUX_MEMREMAP_H_ > + > +#include <linux/mm.h> > #include <linux/range.h> > #include <linux/ioport.h> > #include <linux/percpu-refcount.h> > @@ -129,6 +131,22 @@ static inline unsigned long pgmap_vmemmap_nr(struct dev_pagemap *pgmap) > return 1 << pgmap->vmemmap_shift; > } > > +static inline bool is_device_private_page(const struct page *page) > +{ > + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > + IS_ENABLED(CONFIG_DEVICE_PRIVATE) && > + is_zone_device_page(page) && > + page->pgmap->type == MEMORY_DEVICE_PRIVATE; > +} > + > +static inline bool is_pci_p2pdma_page(const struct page *page) > +{ > + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > + IS_ENABLED(CONFIG_PCI_P2PDMA) && > + is_zone_device_page(page) && > + page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; > +} > + > #ifdef CONFIG_ZONE_DEVICE > void *memremap_pages(struct dev_pagemap *pgmap, int nid); > void memunmap_pages(struct dev_pagemap *pgmap); > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 26baadcef4556b..80fccfe31c3444 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -23,7 +23,6 @@ > #include <linux/err.h> > #include <linux/page-flags.h> > #include <linux/page_ref.h> > -#include <linux/memremap.h> > #include <linux/overflow.h> > #include <linux/sizes.h> > #include <linux/sched.h> > @@ -1101,9 +1100,6 @@ static inline bool put_devmap_managed_page(struct page *page) > return false; > if (!is_zone_device_page(page)) > return false; > - if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && > - page->pgmap->type != MEMORY_DEVICE_FS_DAX) > - return false; > return __put_devmap_managed_page(page); > } > > @@ -1114,22 +1110,6 @@ static inline bool put_devmap_managed_page(struct page *page) > } > #endif /* CONFIG_DEV_PAGEMAP_OPS */ > > -static inline bool is_device_private_page(const struct page *page) > -{ > - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > - IS_ENABLED(CONFIG_DEVICE_PRIVATE) && > - is_zone_device_page(page) && > - page->pgmap->type == MEMORY_DEVICE_PRIVATE; > -} > - > -static inline bool is_pci_p2pdma_page(const struct page *page) > -{ > - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > - IS_ENABLED(CONFIG_PCI_P2PDMA) && > - is_zone_device_page(page) && > - page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; > -} > - > /* 127: arbitrary random number, small enough to assemble well */ > #define folio_ref_zero_or_close_to_overflow(folio) \ > ((unsigned int) folio_ref_count(folio) + 127u <= 127u) > diff --git a/lib/test_hmm.c b/lib/test_hmm.c > index 396beee6b061d4..e5fc14ba71f33e 100644 > --- a/lib/test_hmm.c > +++ b/lib/test_hmm.c > @@ -12,6 +12,7 @@ > #include <linux/kernel.h> > #include <linux/cdev.h> > #include <linux/device.h> > +#include <linux/memremap.h> > #include <linux/mutex.h> > #include <linux/rwsem.h> > #include <linux/sched.h> > diff --git a/mm/memremap.c b/mm/memremap.c > index f41233a67edb12..a0ece2344c2cab 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -4,7 +4,7 @@ > #include <linux/io.h> > #include <linux/kasan.h> > #include <linux/memory_hotplug.h> > -#include <linux/mm.h> > +#include <linux/memremap.h> > #include <linux/pfn_t.h> > #include <linux/swap.h> > #include <linux/mmzone.h> > @@ -504,6 +504,10 @@ void free_devmap_managed_page(struct page *page) > > bool __put_devmap_managed_page(struct page *page) > { > + if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && > + page->pgmap->type != MEMORY_DEVICE_FS_DAX) > + return false; > + > /* > * devmap page refcounts are 1-based, rather than 0-based: if > * refcount is 1, then the page is free and the refcount is
On Sun, Feb 6, 2022 at 10:33 PM Christoph Hellwig <hch@lst.de> wrote: > > Move the check for the actual pgmap types that need the free at refcount > one behavior into the out of line helper, and thus avoid the need to > pull memremap.h into mm.h. Looks good to me assuming the compile bots agree. Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/arm64/mm/mmu.c | 1 + > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 + > drivers/gpu/drm/drm_cache.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + > drivers/gpu/drm/nouveau/nouveau_svm.c | 1 + > drivers/infiniband/core/rw.c | 1 + > drivers/nvdimm/pmem.h | 1 + > drivers/nvme/host/pci.c | 1 + > drivers/nvme/target/io-cmd-bdev.c | 1 + > fs/fuse/virtio_fs.c | 1 + > include/linux/memremap.h | 18 ++++++++++++++++++ > include/linux/mm.h | 20 -------------------- > lib/test_hmm.c | 1 + > mm/memremap.c | 6 +++++- > 14 files changed, 34 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index acfae9b41cc8c9..580abae6c0b93f 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -17,6 +17,7 @@ > #include <linux/mman.h> > #include <linux/nodemask.h> > #include <linux/memblock.h> > +#include <linux/memremap.h> > #include <linux/memory.h> > #include <linux/fs.h> > #include <linux/io.h> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index ea68f3b3a4e9cb..6d643b4b791d87 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -25,6 +25,7 @@ > > #include <linux/hashtable.h> > #include <linux/mmu_notifier.h> > +#include <linux/memremap.h> > #include <linux/mutex.h> > #include <linux/types.h> > #include <linux/atomic.h> > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index f19d9acbe95936..50b8a088f763a6 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -27,11 +27,11 @@ > /* > * Authors: Thomas Hellström <thomas-at-tungstengraphics-dot-com> > */ > - > #include <linux/dma-buf-map.h> > #include <linux/export.h> > #include <linux/highmem.h> > #include <linux/cc_platform.h> > +#include <linux/ioport.h> > #include <xen/xen.h> > > #include <drm/drm_cache.h> > diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c > index e886a3b9e08c7d..a5cdfbe32b5e54 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c > @@ -39,6 +39,7 @@ > > #include <linux/sched/mm.h> > #include <linux/hmm.h> > +#include <linux/memremap.h> > #include <linux/migrate.h> > > /* > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c > index 266809e511e2c1..090b9b47708cca 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c > @@ -35,6 +35,7 @@ > #include <linux/sched/mm.h> > #include <linux/sort.h> > #include <linux/hmm.h> > +#include <linux/memremap.h> > #include <linux/rmap.h> > > struct nouveau_svm { > diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c > index 5a3bd41b331c93..4d98f931a13ddd 100644 > --- a/drivers/infiniband/core/rw.c > +++ b/drivers/infiniband/core/rw.c > @@ -2,6 +2,7 @@ > /* > * Copyright (c) 2016 HGST, a Western Digital Company. > */ > +#include <linux/memremap.h> > #include <linux/moduleparam.h> > #include <linux/slab.h> > #include <linux/pci-p2pdma.h> > diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h > index 59cfe13ea8a85c..1f51a23614299b 100644 > --- a/drivers/nvdimm/pmem.h > +++ b/drivers/nvdimm/pmem.h > @@ -3,6 +3,7 @@ > #define __NVDIMM_PMEM_H__ > #include <linux/page-flags.h> > #include <linux/badblocks.h> > +#include <linux/memremap.h> > #include <linux/types.h> > #include <linux/pfn_t.h> > #include <linux/fs.h> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 6a99ed68091589..ab15bc72710dbe 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -15,6 +15,7 @@ > #include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/memremap.h> > #include <linux/mm.h> > #include <linux/module.h> > #include <linux/mutex.h> > diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c > index 70ca9dfc1771a9..a141446db1bea3 100644 > --- a/drivers/nvme/target/io-cmd-bdev.c > +++ b/drivers/nvme/target/io-cmd-bdev.c > @@ -6,6 +6,7 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > #include <linux/blkdev.h> > #include <linux/blk-integrity.h> > +#include <linux/memremap.h> > #include <linux/module.h> > #include "nvmet.h" > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 9d737904d07c0b..86b7dbb6a0d43e 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -8,6 +8,7 @@ > #include <linux/dax.h> > #include <linux/pci.h> > #include <linux/pfn_t.h> > +#include <linux/memremap.h> > #include <linux/module.h> > #include <linux/virtio.h> > #include <linux/virtio_fs.h> > diff --git a/include/linux/memremap.h b/include/linux/memremap.h > index 1fafcc38acbad6..514ab46f597e5c 100644 > --- a/include/linux/memremap.h > +++ b/include/linux/memremap.h > @@ -1,6 +1,8 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > #ifndef _LINUX_MEMREMAP_H_ > #define _LINUX_MEMREMAP_H_ > + > +#include <linux/mm.h> > #include <linux/range.h> > #include <linux/ioport.h> > #include <linux/percpu-refcount.h> > @@ -129,6 +131,22 @@ static inline unsigned long pgmap_vmemmap_nr(struct dev_pagemap *pgmap) > return 1 << pgmap->vmemmap_shift; > } > > +static inline bool is_device_private_page(const struct page *page) > +{ > + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > + IS_ENABLED(CONFIG_DEVICE_PRIVATE) && > + is_zone_device_page(page) && > + page->pgmap->type == MEMORY_DEVICE_PRIVATE; > +} > + > +static inline bool is_pci_p2pdma_page(const struct page *page) > +{ > + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > + IS_ENABLED(CONFIG_PCI_P2PDMA) && > + is_zone_device_page(page) && > + page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; > +} > + > #ifdef CONFIG_ZONE_DEVICE > void *memremap_pages(struct dev_pagemap *pgmap, int nid); > void memunmap_pages(struct dev_pagemap *pgmap); > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 26baadcef4556b..80fccfe31c3444 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -23,7 +23,6 @@ > #include <linux/err.h> > #include <linux/page-flags.h> > #include <linux/page_ref.h> > -#include <linux/memremap.h> > #include <linux/overflow.h> > #include <linux/sizes.h> > #include <linux/sched.h> > @@ -1101,9 +1100,6 @@ static inline bool put_devmap_managed_page(struct page *page) > return false; > if (!is_zone_device_page(page)) > return false; > - if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && > - page->pgmap->type != MEMORY_DEVICE_FS_DAX) > - return false; > return __put_devmap_managed_page(page); > } > > @@ -1114,22 +1110,6 @@ static inline bool put_devmap_managed_page(struct page *page) > } > #endif /* CONFIG_DEV_PAGEMAP_OPS */ > > -static inline bool is_device_private_page(const struct page *page) > -{ > - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > - IS_ENABLED(CONFIG_DEVICE_PRIVATE) && > - is_zone_device_page(page) && > - page->pgmap->type == MEMORY_DEVICE_PRIVATE; > -} > - > -static inline bool is_pci_p2pdma_page(const struct page *page) > -{ > - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > - IS_ENABLED(CONFIG_PCI_P2PDMA) && > - is_zone_device_page(page) && > - page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; > -} > - > /* 127: arbitrary random number, small enough to assemble well */ > #define folio_ref_zero_or_close_to_overflow(folio) \ > ((unsigned int) folio_ref_count(folio) + 127u <= 127u) > diff --git a/lib/test_hmm.c b/lib/test_hmm.c > index 396beee6b061d4..e5fc14ba71f33e 100644 > --- a/lib/test_hmm.c > +++ b/lib/test_hmm.c > @@ -12,6 +12,7 @@ > #include <linux/kernel.h> > #include <linux/cdev.h> > #include <linux/device.h> > +#include <linux/memremap.h> > #include <linux/mutex.h> > #include <linux/rwsem.h> > #include <linux/sched.h> > diff --git a/mm/memremap.c b/mm/memremap.c > index f41233a67edb12..a0ece2344c2cab 100644 > --- a/mm/memremap.c > +++ b/mm/memremap.c > @@ -4,7 +4,7 @@ > #include <linux/io.h> > #include <linux/kasan.h> > #include <linux/memory_hotplug.h> > -#include <linux/mm.h> > +#include <linux/memremap.h> > #include <linux/pfn_t.h> > #include <linux/swap.h> > #include <linux/mmzone.h> > @@ -504,6 +504,10 @@ void free_devmap_managed_page(struct page *page) > > bool __put_devmap_managed_page(struct page *page) > { > + if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && > + page->pgmap->type != MEMORY_DEVICE_FS_DAX) > + return false; > + > /* > * devmap page refcounts are 1-based, rather than 0-based: if > * refcount is 1, then the page is free and the refcount is > -- > 2.30.2 >
On Mon, Feb 07, 2022 at 04:19:29PM -0500, Felix Kuehling wrote: > > Am 2022-02-07 um 01:32 schrieb Christoph Hellwig: >> Move the check for the actual pgmap types that need the free at refcount >> one behavior into the out of line helper, and thus avoid the need to >> pull memremap.h into mm.h. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > The amdkfd part looks good to me. > > It looks like this patch is not based on Alex Sierra's coherent memory > series. He added two new helpers is_device_coherent_page and > is_dev_private_or_coherent_page that would need to be moved along with > is_device_private_page and is_pci_p2pdma_page. Yes. I Naked that series because it spreads te mess with the refcount further in this latest version. My intent is that it gets rebased on top of this to avoid that spread. Same for the p2p series form Logan.
On Mon, Feb 7, 2022 at 3:49 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Sun, Feb 6, 2022 at 10:33 PM Christoph Hellwig <hch@lst.de> wrote: > > > > Move the check for the actual pgmap types that need the free at refcount > > one behavior into the out of line helper, and thus avoid the need to > > pull memremap.h into mm.h. > > Looks good to me assuming the compile bots agree. > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> Yeah, same as Logan: mm/memcontrol.c: In function ‘get_mctgt_type’: mm/memcontrol.c:5724:29: error: implicit declaration of function ‘is_device_private_page’; did you mean ‘is_device_private_entry’? [-Werror=implicit-function-declaration] 5724 | if (is_device_private_page(page)) | ^~~~~~~~~~~~~~~~~~~~~~ | is_device_private_entry ...needs: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index d1e97a54ae53..0ac7515c85f9 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -62,6 +62,7 @@ #include <linux/tracehook.h> #include <linux/psi.h> #include <linux/seq_buf.h> +#include <linux/memremap.h> #include "internal.h" #include <net/sock.h> #include <net/ip.h>
On Tue, Feb 08, 2022 at 03:53:14PM -0800, Dan Williams wrote: > Yeah, same as Logan: > > mm/memcontrol.c: In function ‘get_mctgt_type’: > mm/memcontrol.c:5724:29: error: implicit declaration of function > ‘is_device_private_page’; did you mean > ‘is_device_private_entry’? [-Werror=implicit-function-declaration] > 5724 | if (is_device_private_page(page)) > | ^~~~~~~~~~~~~~~~~~~~~~ > | is_device_private_entry > > ...needs: Yeah, the buildbot also complained. I've fixed this up locally now.
On Mon, Feb 07, 2022 at 04:19:29PM -0500, Felix Kuehling wrote: > > Am 2022-02-07 um 01:32 schrieb Christoph Hellwig: >> Move the check for the actual pgmap types that need the free at refcount >> one behavior into the out of line helper, and thus avoid the need to >> pull memremap.h into mm.h. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > The amdkfd part looks good to me. > > It looks like this patch is not based on Alex Sierra's coherent memory > series. He added two new helpers is_device_coherent_page and > is_dev_private_or_coherent_page that would need to be moved along with > is_device_private_page and is_pci_p2pdma_page. FYI, here is a branch that contains a rebase of the coherent memory related patches on top of this series: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/pgmap-refcount I don't have a good way to test this, but I'll at least let the build bot finish before sending it out (probably tomorrow).
On Thursday, 10 February 2022 4:48:36 AM AEDT Christoph Hellwig wrote: > On Mon, Feb 07, 2022 at 04:19:29PM -0500, Felix Kuehling wrote: > > > > Am 2022-02-07 um 01:32 schrieb Christoph Hellwig: > >> Move the check for the actual pgmap types that need the free at refcount > >> one behavior into the out of line helper, and thus avoid the need to > >> pull memremap.h into mm.h. > >> > >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > The amdkfd part looks good to me. > > > > It looks like this patch is not based on Alex Sierra's coherent memory > > series. He added two new helpers is_device_coherent_page and > > is_dev_private_or_coherent_page that would need to be moved along with > > is_device_private_page and is_pci_p2pdma_page. > > FYI, here is a branch that contains a rebase of the coherent memory > related patches on top of this series: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/pgmap-refcount > > I don't have a good way to test this, but I'll at least let the build bot > finish before sending it out (probably tomorrow). Thanks, I ran up hmm-test which revealed a few minor problems with the rebase. Fixes below. --- diff --git a/mm/gup.c b/mm/gup.c index cbb49abb7992..8e85c9fb8df4 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2007,7 +2007,6 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, if (!ret && list_empty(&movable_page_list) && !isolation_error_count) return nr_pages; - ret = 0; unpin_pages: for (i = 0; i < nr_pages; i++) if (!pages[i]) diff --git a/mm/migrate.c b/mm/migrate.c index f909f5a92757..1ae3e99baa50 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2686,12 +2686,11 @@ static void migrate_vma_insert_page(struct migrate_vma *migrate, swp_entry = make_readable_device_private_entry( page_to_pfn(page)); entry = swp_entry_to_pte(swp_entry); - } else { - if (is_zone_device_page(page) && - is_device_coherent_page(page)) { + } else if (is_zone_device_page(page) && + !is_device_coherent_page(page)) { pr_warn_once("Unsupported ZONE_DEVICE page type.\n"); goto abort; - } + } else { entry = mk_pte(page, vma->vm_page_prot); if (vma->vm_flags & VM_WRITE) entry = pte_mkwrite(pte_mkdirty(entry));
On Thu, Feb 10, 2022 at 01:10:47PM +1100, Alistair Popple wrote: > diff --git a/mm/gup.c b/mm/gup.c > index cbb49abb7992..8e85c9fb8df4 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2007,7 +2007,6 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, > if (!ret && list_empty(&movable_page_list) && !isolation_error_count) > return nr_pages; > > - ret = 0; > unpin_pages: This isn't quite correct as ret is initially set to -EFAULT now. I'll fix it by removing the early ret initialization and always using the goto. I've also added another refactoring patch for this messy function. I've folded the inversion of the is_device_coherent_page check in migrate.c in as well, thanks!
Am 2022-02-09 um 12:48 schrieb Christoph Hellwig: > On Mon, Feb 07, 2022 at 04:19:29PM -0500, Felix Kuehling wrote: >> Am 2022-02-07 um 01:32 schrieb Christoph Hellwig: >>> Move the check for the actual pgmap types that need the free at refcount >>> one behavior into the out of line helper, and thus avoid the need to >>> pull memremap.h into mm.h. >>> >>> Signed-off-by: Christoph Hellwig <hch@lst.de> >> The amdkfd part looks good to me. >> >> It looks like this patch is not based on Alex Sierra's coherent memory >> series. He added two new helpers is_device_coherent_page and >> is_dev_private_or_coherent_page that would need to be moved along with >> is_device_private_page and is_pci_p2pdma_page. > FYI, here is a branch that contains a rebase of the coherent memory > related patches on top of this series: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/pgmap-refcount > > I don't have a good way to test this, but I'll at least let the build bot > finish before sending it out (probably tomorrow). Thank you for taking care of this rebase! Alex tested it on one of our coherent memory systems and it passed our tests. I see you also included these rebased patches in your latest 27-patch series. I'll try to review the changes in more detail over the weekend. Regards, Felix
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index acfae9b41cc8c9..580abae6c0b93f 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -17,6 +17,7 @@ #include <linux/mman.h> #include <linux/nodemask.h> #include <linux/memblock.h> +#include <linux/memremap.h> #include <linux/memory.h> #include <linux/fs.h> #include <linux/io.h> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index ea68f3b3a4e9cb..6d643b4b791d87 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -25,6 +25,7 @@ #include <linux/hashtable.h> #include <linux/mmu_notifier.h> +#include <linux/memremap.h> #include <linux/mutex.h> #include <linux/types.h> #include <linux/atomic.h> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index f19d9acbe95936..50b8a088f763a6 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -27,11 +27,11 @@ /* * Authors: Thomas Hellström <thomas-at-tungstengraphics-dot-com> */ - #include <linux/dma-buf-map.h> #include <linux/export.h> #include <linux/highmem.h> #include <linux/cc_platform.h> +#include <linux/ioport.h> #include <xen/xen.h> #include <drm/drm_cache.h> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c index e886a3b9e08c7d..a5cdfbe32b5e54 100644 --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c @@ -39,6 +39,7 @@ #include <linux/sched/mm.h> #include <linux/hmm.h> +#include <linux/memremap.h> #include <linux/migrate.h> /* diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 266809e511e2c1..090b9b47708cca 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -35,6 +35,7 @@ #include <linux/sched/mm.h> #include <linux/sort.h> #include <linux/hmm.h> +#include <linux/memremap.h> #include <linux/rmap.h> struct nouveau_svm { diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c index 5a3bd41b331c93..4d98f931a13ddd 100644 --- a/drivers/infiniband/core/rw.c +++ b/drivers/infiniband/core/rw.c @@ -2,6 +2,7 @@ /* * Copyright (c) 2016 HGST, a Western Digital Company. */ +#include <linux/memremap.h> #include <linux/moduleparam.h> #include <linux/slab.h> #include <linux/pci-p2pdma.h> diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h index 59cfe13ea8a85c..1f51a23614299b 100644 --- a/drivers/nvdimm/pmem.h +++ b/drivers/nvdimm/pmem.h @@ -3,6 +3,7 @@ #define __NVDIMM_PMEM_H__ #include <linux/page-flags.h> #include <linux/badblocks.h> +#include <linux/memremap.h> #include <linux/types.h> #include <linux/pfn_t.h> #include <linux/fs.h> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 6a99ed68091589..ab15bc72710dbe 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -15,6 +15,7 @@ #include <linux/init.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/memremap.h> #include <linux/mm.h> #include <linux/module.h> #include <linux/mutex.h> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c index 70ca9dfc1771a9..a141446db1bea3 100644 --- a/drivers/nvme/target/io-cmd-bdev.c +++ b/drivers/nvme/target/io-cmd-bdev.c @@ -6,6 +6,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/blkdev.h> #include <linux/blk-integrity.h> +#include <linux/memremap.h> #include <linux/module.h> #include "nvmet.h" diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 9d737904d07c0b..86b7dbb6a0d43e 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -8,6 +8,7 @@ #include <linux/dax.h> #include <linux/pci.h> #include <linux/pfn_t.h> +#include <linux/memremap.h> #include <linux/module.h> #include <linux/virtio.h> #include <linux/virtio_fs.h> diff --git a/include/linux/memremap.h b/include/linux/memremap.h index 1fafcc38acbad6..514ab46f597e5c 100644 --- a/include/linux/memremap.h +++ b/include/linux/memremap.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0 */ #ifndef _LINUX_MEMREMAP_H_ #define _LINUX_MEMREMAP_H_ + +#include <linux/mm.h> #include <linux/range.h> #include <linux/ioport.h> #include <linux/percpu-refcount.h> @@ -129,6 +131,22 @@ static inline unsigned long pgmap_vmemmap_nr(struct dev_pagemap *pgmap) return 1 << pgmap->vmemmap_shift; } +static inline bool is_device_private_page(const struct page *page) +{ + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && + IS_ENABLED(CONFIG_DEVICE_PRIVATE) && + is_zone_device_page(page) && + page->pgmap->type == MEMORY_DEVICE_PRIVATE; +} + +static inline bool is_pci_p2pdma_page(const struct page *page) +{ + return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && + IS_ENABLED(CONFIG_PCI_P2PDMA) && + is_zone_device_page(page) && + page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; +} + #ifdef CONFIG_ZONE_DEVICE void *memremap_pages(struct dev_pagemap *pgmap, int nid); void memunmap_pages(struct dev_pagemap *pgmap); diff --git a/include/linux/mm.h b/include/linux/mm.h index 26baadcef4556b..80fccfe31c3444 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -23,7 +23,6 @@ #include <linux/err.h> #include <linux/page-flags.h> #include <linux/page_ref.h> -#include <linux/memremap.h> #include <linux/overflow.h> #include <linux/sizes.h> #include <linux/sched.h> @@ -1101,9 +1100,6 @@ static inline bool put_devmap_managed_page(struct page *page) return false; if (!is_zone_device_page(page)) return false; - if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && - page->pgmap->type != MEMORY_DEVICE_FS_DAX) - return false; return __put_devmap_managed_page(page); } @@ -1114,22 +1110,6 @@ static inline bool put_devmap_managed_page(struct page *page) } #endif /* CONFIG_DEV_PAGEMAP_OPS */ -static inline bool is_device_private_page(const struct page *page) -{ - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && - IS_ENABLED(CONFIG_DEVICE_PRIVATE) && - is_zone_device_page(page) && - page->pgmap->type == MEMORY_DEVICE_PRIVATE; -} - -static inline bool is_pci_p2pdma_page(const struct page *page) -{ - return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && - IS_ENABLED(CONFIG_PCI_P2PDMA) && - is_zone_device_page(page) && - page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA; -} - /* 127: arbitrary random number, small enough to assemble well */ #define folio_ref_zero_or_close_to_overflow(folio) \ ((unsigned int) folio_ref_count(folio) + 127u <= 127u) diff --git a/lib/test_hmm.c b/lib/test_hmm.c index 396beee6b061d4..e5fc14ba71f33e 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -12,6 +12,7 @@ #include <linux/kernel.h> #include <linux/cdev.h> #include <linux/device.h> +#include <linux/memremap.h> #include <linux/mutex.h> #include <linux/rwsem.h> #include <linux/sched.h> diff --git a/mm/memremap.c b/mm/memremap.c index f41233a67edb12..a0ece2344c2cab 100644 --- a/mm/memremap.c +++ b/mm/memremap.c @@ -4,7 +4,7 @@ #include <linux/io.h> #include <linux/kasan.h> #include <linux/memory_hotplug.h> -#include <linux/mm.h> +#include <linux/memremap.h> #include <linux/pfn_t.h> #include <linux/swap.h> #include <linux/mmzone.h> @@ -504,6 +504,10 @@ void free_devmap_managed_page(struct page *page) bool __put_devmap_managed_page(struct page *page) { + if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && + page->pgmap->type != MEMORY_DEVICE_FS_DAX) + return false; + /* * devmap page refcounts are 1-based, rather than 0-based: if * refcount is 1, then the page is free and the refcount is
Move the check for the actual pgmap types that need the free at refcount one behavior into the out of line helper, and thus avoid the need to pull memremap.h into mm.h. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/arm64/mm/mmu.c | 1 + drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 + drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + drivers/gpu/drm/nouveau/nouveau_svm.c | 1 + drivers/infiniband/core/rw.c | 1 + drivers/nvdimm/pmem.h | 1 + drivers/nvme/host/pci.c | 1 + drivers/nvme/target/io-cmd-bdev.c | 1 + fs/fuse/virtio_fs.c | 1 + include/linux/memremap.h | 18 ++++++++++++++++++ include/linux/mm.h | 20 -------------------- lib/test_hmm.c | 1 + mm/memremap.c | 6 +++++- 14 files changed, 34 insertions(+), 22 deletions(-)