Fix kvm misconceives NVDIMM pages as reserved mmio
Subsystems (6)
v5 cover Fix kvm misconceives NVDIMM pages as reserved mmio
For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved. the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to indentify these pages are from NVDIMM pmem. and let kvm treat these as normal pages.
Without this patch, Many operations will be missed due to this mistreatment to pmem pages. For example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean); not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.
V1:
https://lkml.org/lkml/2018/7/4/91
V2:
https://lkml.org/lkml/2018/7/10/135
V3:
https://lkml.org/lkml/2018/8/9/17
V4:
https://lkml.org/lkml/2018/8/22/17
V5:
[PATCH V3 1/4] Reviewed-by: David / Acked-by: Pankaj [PATCH V3 2/4] Reviewed-by: Jan
[PATCH V3 3/4] Acked-by: Jan
[PATCH V3 4/4] Added "Acked-by: Pankaj", Added in-line comments: Dave
Zhang Yi (4):
kvm: remove redundant reserved page check
mm: introduce memory type MEMORY_DEVICE_DEV_DAX
mm: add a function to differentiate the pages is from DAX device
memory
kvm: add a check if pfn is from NVDIMM pmem.
drivers/dax/pmem.c | 1 + include/linux/memremap.h | 8 ++++++++ include/linux/mm.h | 12 ++++++++++++ virt/kvm/kvm_main.c | 24 ++++++++++++++++-------- 4 files changed, 37 insertions(+), 8 deletions(-)
Signature · 6 lines
2.7.4 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
PATCH v5 1/4 kvm: remove redundant reserved page check
PageReserved() is already checked inside kvm_is_reserved_pfn(), remove it from kvm_set_pfn_dirty().
--- virt/kvm/kvm_main.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
virt/kvm/kvm_main.c +2 -6
@@ -1690,12 +1690,8 @@ EXPORT_SYMBOL_GPL(kvm_release_pfn_dirty);void kvm_set_pfn_dirty (kvm_pfn_t pfn) { -if (!kvm_is_reserved_pfn (pfn)) { -struct page *page =pfn_to_page (pfn); - -if (!PageReserved (page)) -SetPageDirty (page); - } +if (!kvm_is_reserved_pfn (pfn)) +SetPageDirty (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_set_pfn_dirty);
Signature · 6 lines
2.7.4 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V5 1/4] kvm: remove redundant reserved page check
Quoted 33 lines · Zhang Yi
PageReserved()is already checked insidekvm_is_reserved_pfn(), remove it fromkvm_set_pfn_dirty().Signed-off-by Zhang Yi <[email protected]>Signed-off-by Zhang Yu <[email protected]>Reviewed-by David Hildenbrand <[email protected]>Acked-by Pankaj Gupta <[email protected]>--- virt/kvm/kvm_main.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)virt/kvm/kvm_main.c +2
-6@@ -1690,12 +1690,8 @@ EXPORT_SYMBOL_GPL(kvm_release_pfn_dirty);void kvm_set_pfn_dirty (kvm_pfn_t pfn) { -if (!kvm_is_reserved_pfn (pfn)) { -struct page *page =pfn_to_page (pfn); - -if (!PageReserved (page)) -SetPageDirty (page); - } +if (!kvm_is_reserved_pfn (pfn)) +SetPageDirty (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_set_pfn_dirty);Signature · 1 line
2.7.4
Hi Paolo,
We will remove the reserved flag in dax pages, then patch 2[3,4]/4 is unnecessary, can we queue this 1/4 to next merge?
Thank you very much.
Yi
PATCH v5 2/4 mm: introduce memory type MEMORY_DEVICE_DEV_DAX
In reply to Zhang YiCurrently, NVDIMM pages will be marked 'PageReserved'. However, unlike other reserved PFNs, pages on NVDIMM shall still behave like normal ones in many cases, i.e. when used as backend memory of KVM guest. This patch introduces a new memory type, MEMORY_DEVICE_DEV_DAX. And set this flag while dax driver hotplug the device memory.
--- drivers/dax/pmem.c | 1 + include/linux/memremap.h | 8 ++++++++ 2 files changed, 9 insertions(+)
drivers/dax/pmem.c +1 -0
@@ -111,6 +111,7 @@ static int dax_pmem_probe(struct device *dev)return rc; dax_pmem->pgmap.ref = &dax_pmem->ref; + dax_pmem->pgmap.type =MEMORY_DEVICE_DEV_DAX ; addr =devm_memremap_pages (dev, &dax_pmem->pgmap);if (IS_ERR (addr))return PTR_ERR (addr);
include/linux/memremap.h +8 -0
@@ -53,11 +53,19 @@ struct vmem_altmap {* wakeup event whenever a page is unpinned and becomes idle. This * wakeup is used to coordinate physical address space management (ex: * fs truncate/hole punch) vs pinned pages (ex: device dma). +* +* MEMORY_DEVICE_DEV_DAX: +* Device memory that support raw access to persistent memory. Without need +* of an intervening filesystem, it could be directed mapped via an mmap +* capable character device. Together with the type MEMORY_DEVICE_FS_DAX, +* we could distinguish the persistent memory pages from normal ZONE_DEVICE +* pages. */ enum memory_type {MEMORY_DEVICE_PRIVATE =1 ,MEMORY_DEVICE_PUBLIC ,MEMORY_DEVICE_FS_DAX , +MEMORY_DEVICE_DEV_DAX , };/*
Signature · 6 lines
2.7.4 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
PATCH v5 3/4 mm: add a function to differentiate the pages is from DAX device memory
In reply to Zhang YiDAX driver hotplug the device memory and move it to memory zone, these pages will be marked reserved flag, however, some other kernel componet will misconceive these pages are reserved mmio (ex: we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type MEMORY_DEVICE_FS_DAX, we can use is_dax_page() to differentiate the pages is DAX device memory or not.
--- include/linux/mm.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
include/linux/mm.h +12 -0
@@ -889,6 +889,13 @@ static inline bool is_device_public_page(const struct page *page) page->pgmap->type == MEMORY_DEVICE_PUBLIC; } +static inline bool is_dax_page (const struct page *page) +{ +return is_zone_device_page (page) && + (page->pgmap->type ==MEMORY_DEVICE_FS_DAX || + page->pgmap->type ==MEMORY_DEVICE_DEV_DAX ); +} +#else /* CONFIG_DEV_PAGEMAP_OPS */ static inline void dev_pagemap_get_ops (void ) {
@@ -912,6 +919,11 @@ static inline bool is_device_public_page(const struct page *page) {return false; } + +static inline bool is_dax_page (const struct page *page) +{ +return false; +}#endif /* CONFIG_DEV_PAGEMAP_OPS */ static inline void get_page (struct page *page)
Signature · 6 lines
2.7.4 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
PATCH v5 4/4 kvm: add a check if pfn is from NVDIMM pmem.
In reply to Zhang YiFor device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.
Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.
--- virt/kvm/kvm_main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +14 -2
@@ -147,8 +147,20 @@ __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,bool kvm_is_reserved_pfn (kvm_pfn_t pfn) { -if (pfn_valid (pfn)) -return PageReserved (pfn_to_page (pfn)); +struct page *page; + +if (pfn_valid (pfn)) { + page =pfn_to_page (pfn); + +/* +* For device specific memory space, there is a case +* which we need pass MEMORY_DEVICE_FS[DEV]_DAX pages +* to kvm, these pages marked reserved flag as it is a +* zone device memory, we need to identify these pages +* and let kvm treat these as normal pages +*/ +return PageReserved (page) && !is_dax_page (page); + }return true; }
Signature · 6 lines
2.7.4 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 37 lines · Zhang Yi
For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these
dev_daxorfs_daxpages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check ofkvm_is_reserved_pfn()misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM
guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.Signed-off-by Zhang Yi <[email protected]>Acked-by Pankaj Gupta <[email protected]>--- virt/kvm/kvm_main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)virt/kvm/kvm_main.c +3
-2@@ -147,8 +147,20 @@ __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,bool kvm_is_reserved_pfn (kvm_pfn_t pfn) { -if (pfn_valid (pfn)) -return PageReserved (pfn_to_page (pfn)); +struct page *page; + +if (pfn_valid (pfn)) {
virt/kvm/kvm_main.c __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm, · line 162
Quoted 10 lines
virt/kvm/kvm_main.c +10
-0+ page =pfn_to_page (pfn); + +/* +* For device specific memory space, there is a case +* which we need pass MEMORY_DEVICE_FS[DEV]_DAX pages +* to kvm, these pages marked reserved flag as it is a +* zone device memory, we need to identify these pages +* and let kvm treat these as normal pages +*/ +return PageReserved (page) && !is_dax_page (page);
Should we consider just not setting PageReserved for devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?
Why is MEMORY_DEVICE_PUBLIC memory specifically excluded?
This has less to do with "dax" pages and more to do with devm_memremap_pages() established ranges. P2PDMA is another producer of these pages. If either MEMORY_DEVICE_PUBLIC or P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.
That said I haven't audited all the locations that test PageReserved().
Sorry for not responding sooner I was on extended leave.
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Am 19.09.18 um 04:53 schrieb Dan Williams:
Quoted 44 lines · Zhang Yi
Quoted 43 lines
For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these
dev_daxorfs_daxpages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check ofkvm_is_reserved_pfn()misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM
guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.Signed-off-by Zhang Yi <[email protected]>Acked-by Pankaj Gupta <[email protected]>--- virt/kvm/kvm_main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)virt/kvm/kvm_main.c +9
-2@@ -147,8 +147,20 @@ __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,bool kvm_is_reserved_pfn (kvm_pfn_t pfn) { -if (pfn_valid (pfn)) -return PageReserved (pfn_to_page (pfn)); +struct page *page; + +if (pfn_valid (pfn)) { + page =pfn_to_page (pfn); + +/* +* For device specific memory space, there is a case +* which we need pass MEMORY_DEVICE_FS[DEV]_DAX pages +* to kvm, these pages marked reserved flag as it is a
Quoted 8 lines
virt/kvm/kvm_main.c __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm, · line 162
Quoted 4 lines
virt/kvm/kvm_main.c +4
-0+* zone device memory, we need to identify these pages +* and let kvm treat these as normal pages +*/ +return PageReserved (page) && !is_dax_page (page);Should we consider just not setting PageReserved for
devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?
I was asking the exact same question in v3 or so.
I was recently going through all PageReserved users, trying to clean up and document how it is used.
PG_reserved used to be a marker "not available for the page allocator". This is only partially true and not really helpful I think. My current understanding:
"PG_reserved is set for special pages, struct pages of such pages should in general not be touched except by their owner. Pages marked as reserved include:
- Kernel image (including vDSO) and similar (e.g. BIOS, initrd)
- Pages allocated early during boot (bootmem, memblock)
- Zero pages
- Pages that have been associated with a zone but were not onlined (e.g. NVDIMM/pmem, online_page_callback used by XEN)
- Pages to exclude from the hibernation image (e.g. loaded kexec images)
- MCA (memory error) pages on ia64
- Offline pages
Some architectures don't allow to ioremap RAM pages that are not marked as reserved. Allocated pages might have to be set reserved to allow for that - if there is a good reason to enforce this. Consequently, PG_reserved part of a user space table might be the indicator for the zero page, pmem or MMIO pages.
"
Swapping code does not care about PageReserved at all as far as I remember. This seems to be fine as it only looks at the way pages have been mapped into user space.
I don't really see a good reason to set pmem pages as reserved. One question would be, how/if to exclude them from the hibernation image. But that could also be solved differently (we would have to double check how they are handled in hibernation code).
A similar user of PageReserved to look at is:
drivers/vfio/vfio_iommu_type1.c:is_invalid_reserved_pfn()
It will not mark pages dirty if they are reserved. Similar to KVM code.
Quoted 13 lines
Why is
MEMORY_DEVICE_PUBLICmemory specifically excluded?This has less to do with "dax" pages and more to do with
devm_memremap_pages()established ranges. P2PDMA is another producer of these pages. If eitherMEMORY_DEVICE_PUBLICor P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.That said I haven't audited all the locations that test
PageReserved().Sorry for not responding sooner I was on extended leave.
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 44 lines · David Hildenbrand
Am 19.09.18 um 04:53 schrieb Dan Williams:
Quoted 4 lines
Should we consider just not setting PageReserved for
devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?I was asking the exact same question in v3 or so.
I was recently going through all PageReserved users, trying to clean up and document how it is used.
PG_reservedused to be a marker "not available for the page allocator". This is only partially true and not really helpful I think. My current understanding:"
PG_reservedis set for special pages, struct pages of such pages should in general not be touched except by their owner. Pages marked as reserved include:
- Kernel image (including vDSO) and similar (e.g. BIOS, initrd)
- Pages allocated early during boot (bootmem, memblock)
- Zero pages
- Pages that have been associated with a zone but were not onlined (e.g. NVDIMM/pmem,online_page_callbackused by XEN)
- Pages to exclude from the hibernation image (e.g. loaded kexec images)
- MCA (memory error) pages on ia64
- Offline pages
Some architectures don't allow to ioremap RAM pages that are not marked as reserved. Allocated pages might have to be set reserved to allow for that - if there is a good reason to enforce this. Consequently,PG_reservedpart of a user space table might be the indicator for the zero page, pmem or MMIO pages.
"Swapping code does not care about PageReserved at all as far as I remember. This seems to be fine as it only looks at the way pages have been mapped into user space.
I don't really see a good reason to set pmem pages as reserved. One question would be, how/if to exclude them from the hibernation image. But that could also be solved differently (we would have to double check how they are handled in hibernation code).
Quoted 5 lines
A similar user of PageReserved to look at is:
drivers/vfio/vfio_iommu_type1.c:is_invalid_reserved_pfn()
It will not mark pages dirty if they are reserved. Similar to KVM code.
Yes, kvm is not the only one user of the dax reserved page.
Quoted 9 lines
Quoted 9 lines
Why is
MEMORY_DEVICE_PUBLICmemory specifically excluded?This has less to do with "dax" pages and more to do with
devm_memremap_pages()established ranges. P2PDMA is another producer of these pages. If eitherMEMORY_DEVICE_PUBLICor P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.
Thanks Dan/David's comments.
for MEMORY_DEVICE_PUBLIC memory, since host driver could manager the memory resource to share to guest, Jerome says we could ignore it at this time.
And p2pmem, it seems mapped in a PCI bar space which should most likely a mmio. I think kvm should treated as a reserved page.
Quoted 12 lines
Quoted 5 lines
That said I haven't audited all the locations that test
PageReserved().Sorry for not responding sooner I was on extended leave.
Signature · 3 lines
Thanks, David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 62 lines · Yi Zhang
Quoted 44 lines · David Hildenbrand
Am 19.09.18 um 04:53 schrieb Dan Williams:
Quoted 4 lines
Should we consider just not setting PageReserved for
devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?I was asking the exact same question in v3 or so.
I was recently going through all PageReserved users, trying to clean up and document how it is used.
PG_reservedused to be a marker "not available for the page allocator". This is only partially true and not really helpful I think. My current understanding:"
PG_reservedis set for special pages, struct pages of such pages should in general not be touched except by their owner. Pages marked as reserved include:
- Kernel image (including vDSO) and similar (e.g. BIOS, initrd)
- Pages allocated early during boot (bootmem, memblock)
- Zero pages
- Pages that have been associated with a zone but were not onlined (e.g. NVDIMM/pmem,online_page_callbackused by XEN)
- Pages to exclude from the hibernation image (e.g. loaded kexec images)
- MCA (memory error) pages on ia64
- Offline pages
Some architectures don't allow to ioremap RAM pages that are not marked as reserved. Allocated pages might have to be set reserved to allow for that - if there is a good reason to enforce this. Consequently,PG_reservedpart of a user space table might be the indicator for the zero page, pmem or MMIO pages.
"Swapping code does not care about PageReserved at all as far as I remember. This seems to be fine as it only looks at the way pages have been mapped into user space.
I don't really see a good reason to set pmem pages as reserved. One question would be, how/if to exclude them from the hibernation image. But that could also be solved differently (we would have to double check how they are handled in hibernation code).
Quoted 5 lines
A similar user of PageReserved to look at is:
drivers/vfio/vfio_iommu_type1.c:is_invalid_reserved_pfn()
It will not mark pages dirty if they are reserved. Similar to KVM code.
Yes, kvm is not the only one user of the dax reserved page.
Quoted 9 lines
Quoted 9 lines
Why is
MEMORY_DEVICE_PUBLICmemory specifically excluded?This has less to do with "dax" pages and more to do with
devm_memremap_pages()established ranges. P2PDMA is another producer of these pages. If eitherMEMORY_DEVICE_PUBLICor P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.
Quoted 7 lines
Thanks Dan/David's comments.
forMEMORY_DEVICE_PUBLICmemory, since host driver could manager the memory resource to share to guest, Jerome says we could ignore it at this time.And p2pmem, it seems mapped in a PCI bar space which should most likely a mmio. I think kvm should treated as a reserved page.
Ok, but the question you left unanswered is whether it would be better for devm_memremap_pages() to clear the PageReserved flag for MEMORY_DEVICE_{FS,DEV}_DAX rather than introduce a local kvm-only hack for what looks like a global problem.
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 68 lines · Dan Williams
Quoted 67 lines · Yi Zhang
Quoted 44 lines · David Hildenbrand
Am 19.09.18 um 04:53 schrieb Dan Williams:
Quoted 4 lines
Should we consider just not setting PageReserved for
devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?I was asking the exact same question in v3 or so.
I was recently going through all PageReserved users, trying to clean up and document how it is used.
PG_reservedused to be a marker "not available for the page allocator". This is only partially true and not really helpful I think. My current understanding:"
PG_reservedis set for special pages, struct pages of such pages should in general not be touched except by their owner. Pages marked as reserved include:
- Kernel image (including vDSO) and similar (e.g. BIOS, initrd)
- Pages allocated early during boot (bootmem, memblock)
- Zero pages
- Pages that have been associated with a zone but were not onlined (e.g. NVDIMM/pmem,online_page_callbackused by XEN)
- Pages to exclude from the hibernation image (e.g. loaded kexec images)
- MCA (memory error) pages on ia64
- Offline pages
Some architectures don't allow to ioremap RAM pages that are not marked as reserved. Allocated pages might have to be set reserved to allow for that - if there is a good reason to enforce this. Consequently,PG_reservedpart of a user space table might be the indicator for the zero page, pmem or MMIO pages.
"Swapping code does not care about PageReserved at all as far as I remember. This seems to be fine as it only looks at the way pages have been mapped into user space.
I don't really see a good reason to set pmem pages as reserved. One question would be, how/if to exclude them from the hibernation image. But that could also be solved differently (we would have to double check how they are handled in hibernation code).
Quoted 5 lines
A similar user of PageReserved to look at is:
drivers/vfio/vfio_iommu_type1.c:is_invalid_reserved_pfn()
It will not mark pages dirty if they are reserved. Similar to KVM code.
Yes, kvm is not the only one user of the dax reserved page.
Quoted 9 lines
Quoted 9 lines
Why is
MEMORY_DEVICE_PUBLICmemory specifically excluded?This has less to do with "dax" pages and more to do with
devm_memremap_pages()established ranges. P2PDMA is another producer of these pages. If eitherMEMORY_DEVICE_PUBLICor P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.Thanks Dan/David's comments.
forMEMORY_DEVICE_PUBLICmemory, since host driver could manager the memory resource to share to guest, Jerome says we could ignore it at this time.
Quoted 7 lines
Quoted 2 lines
And p2pmem, it seems mapped in a PCI bar space which should most likely a mmio. I think kvm should treated as a reserved page.
Ok, but the question you left unanswered is whether it would be better for
devm_memremap_pages()to clear the PageReserved flag for MEMORY_DEVICE_{FS,DEV}_DAX rather than introduce a local kvm-only hack for what looks like a global problem.
Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?
Quoted 4 lines
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 72 lines · Yi Zhang
Quoted 71 lines · Dan Williams
Quoted 69 lines · Yi Zhang
Quoted 44 lines · David Hildenbrand
Am 19.09.18 um 04:53 schrieb Dan Williams:
Quoted 4 lines
Should we consider just not setting PageReserved for
devm_memremap_pages()? Perhaps kvm is not be the only component making these assumptions about this flag?I was asking the exact same question in v3 or so.
I was recently going through all PageReserved users, trying to clean up and document how it is used.
PG_reservedused to be a marker "not available for the page allocator". This is only partially true and not really helpful I think. My current understanding:"
PG_reservedis set for special pages, struct pages of such pages should in general not be touched except by their owner. Pages marked as reserved include:
- Kernel image (including vDSO) and similar (e.g. BIOS, initrd)
- Pages allocated early during boot (bootmem, memblock)
- Zero pages
- Pages that have been associated with a zone but were not onlined (e.g. NVDIMM/pmem,online_page_callbackused by XEN)
- Pages to exclude from the hibernation image (e.g. loaded kexec images)
- MCA (memory error) pages on ia64
- Offline pages
Some architectures don't allow to ioremap RAM pages that are not marked as reserved. Allocated pages might have to be set reserved to allow for that - if there is a good reason to enforce this. Consequently,PG_reservedpart of a user space table might be the indicator for the zero page, pmem or MMIO pages.
"Swapping code does not care about PageReserved at all as far as I remember. This seems to be fine as it only looks at the way pages have been mapped into user space.
I don't really see a good reason to set pmem pages as reserved. One question would be, how/if to exclude them from the hibernation image. But that could also be solved differently (we would have to double check how they are handled in hibernation code).
Quoted 5 lines
A similar user of PageReserved to look at is:
drivers/vfio/vfio_iommu_type1.c:is_invalid_reserved_pfn()
It will not mark pages dirty if they are reserved. Similar to KVM code.
Yes, kvm is not the only one user of the dax reserved page.
Quoted 9 lines
Quoted 9 lines
Why is
MEMORY_DEVICE_PUBLICmemory specifically excluded?This has less to do with "dax" pages and more to do with
devm_memremap_pages()established ranges. P2PDMA is another producer of these pages. If eitherMEMORY_DEVICE_PUBLICor P2PDMA pages can be used in these kvm paths then I think this points to consider clearing the Reserved flag.Thanks Dan/David's comments.
forMEMORY_DEVICE_PUBLICmemory, since host driver could manager the memory resource to share to guest, Jerome says we could ignore it at this time.And p2pmem, it seems mapped in a PCI bar space which should most likely a mmio. I think kvm should treated as a reserved page.
Quoted 8 lines
Quoted 4 lines
Ok, but the question you left unanswered is whether it would be better for
devm_memremap_pages()to clear the PageReserved flag for MEMORY_DEVICE_{FS,DEV}_DAX rather than introduce a local kvm-only hack for what looks like a global problem.Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?
We could use a page type for that or what you proposed. (as I said, we might have to change hibernation code to skip the pages once we drop the reserved flag).
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
On Fri, Sep 21, 2018 at 7:24 AM David Hildenbrand <[email protected]> wrote:
[..]
Quoted 7 lines
Quoted 3 lines
Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?We could use a page type for that or what you proposed. (as I said, we might have to change hibernation code to skip the pages once we drop the reserved flag).
I think it would be reasonable to reject all ZONE_DEVICE pages in saveable_page().
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 6 lines · Dan Williams
On Fri, Sep 21, 2018 at 7:24 AM David Hildenbrand <[email protected]> wrote:
[..]Quoted 4 lines
Quoted 3 lines
Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?
Quoted 7 lines
Quoted 3 lines
We could use a page type for that or what you proposed. (as I said, we might have to change hibernation code to skip the pages once we drop the reserved flag).
I think it would be reasonable to reject all
ZONE_DEVICEpages insaveable_page().
Indeed, that sounds like the easiest solution - guess that answer was too easy for me to figure out :) .
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 11 lines · On 2018-09-21 at 21:29 David Hildenbrand…
Quoted 10 lines · Dan Williams
On Fri, Sep 21, 2018 at 7:24 AM David Hildenbrand <[email protected]> wrote:
[..]Quoted 7 lines
Quoted 3 lines
Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?We could use a page type for that or what you proposed. (as I said, we might have to change hibernation code to skip the pages once we drop the reserved flag).
Quoted 7 lines
Quoted 3 lines
I think it would be reasonable to reject all
ZONE_DEVICEpages insaveable_page().Indeed, that sounds like the easiest solution - guess that answer was too easy for me to figure out :) .
Just to follow-up, is the plan to clear PageReserved for nvdimm pages instead of the approach taken in this patch set? Or should we special case nvdimm/dax pages in kvm_is_reserved_pfn()?
Thanks,
Barret
Re: [PATCH V5 4/4] kvm: add a check if pfn is from NVDIMM pmem.
Quoted 16 lines · Barret Rhoden
Quoted 15 lines · On 2018-09-21 at 21:29 David Hildenbrand <[email protected]> wrote:
Quoted 13 lines · Dan Williams
On Fri, Sep 21, 2018 at 7:24 AM David Hildenbrand <[email protected]> wrote:
[..]Quoted 7 lines
Quoted 3 lines
Remove the PageReserved flag sounds more reasonable.
And Could we still have a flag to identify it is a device private memory, or where these pages coming from?We could use a page type for that or what you proposed. (as I said, we might have to change hibernation code to skip the pages once we drop the reserved flag).
I think it would be reasonable to reject all
ZONE_DEVICEpages insaveable_page().
Quoted 7 lines
Quoted 3 lines
Indeed, that sounds like the easiest solution - guess that answer was too easy for me to figure out :) .
Just to follow-up, is the plan to clear PageReserved for nvdimm pages instead of the approach taken in this patch set? Or should we special case nvdimm/dax pages in
kvm_is_reserved_pfn()?
Yes, we are going to remove the PageReserved flag for nvdimm pages. Added Alex, attached the patch-set.
Quoted 7 lines
Thanks,
Barret
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Hi!
On Sat, Sep 08, 2018 at 02:03:02AM +0800, Zhang Yi wrote:
[...]
Quoted 4 lines
Quoted 9 lines
V2:
https://lkml.org/lkml/2018/7/10/135
Can we please avoid referencing "lkml.org"?
It's just an unreliable broken website. [1][2] Much more important though is that its URLs _hide_ the Message-Id field; running the threat of losing the e-mail reference forever at some point in the future.
Quoted 1 line
From
Documentation/process/submitting-patches.rst:
If the patch follows from a mailing list discussion, give a URL to the mailing list archive; use the https://lkml.kernel.org/
redirector with a ``Message-Id``, to ensure that the links cannot become stale.
So the V1 link above should've been either:
https://lore.kernel.org/lkml/[email protected]
or:
https://lkml.kernel.org/r/[email protected]
and so on..
Thanks,
[1] https://www.theregister.co.uk/2018/01/14/linux_kernel_mailing_list_archives_will_return_soon
[2] The threading interface is also broken and in a lot of cases
does not show all messages in a thread
Thanks Darwi's remind, Will follow that next time.
Thanks.
Yi
Quoted 44 lines · Ahmed S. Darwish
Hi!
On Sat, Sep 08, 2018 at 02:03:02AM +0800, Zhang Yi wrote:
[...]Quoted 4 lines
Quoted 9 lines
V2:
https://lkml.org/lkml/2018/7/10/135Can we please avoid referencing "lkml.org"?
It's just an unreliable broken website. [1][2] Much more important though is that its URLs _hide_ the Message-Id field; running the threat of losing the e-mail reference forever at some point in the future.
From
Documentation/process/submitting-patches.rst:If the patch follows from a mailing list discussion, give a URL to the mailing list archive; use the https://lkml.kernel.org/
redirector with a ``Message-Id``, to ensure that the links cannot become stale.So the V1 link above should've been either:
https://lore.kernel.org/lkml/[email protected]or:
https://lkml.kernel.org/r/[email protected]and so on..
Thanks,
Quoted 7 lines
[1] https://www.theregister.co.uk/2018/01/14/linux_kernel_mailing_list_archives_will_return_soon
[2] The threading interface is also broken and in a lot of cases
does not show all messages in a thread
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Any comments?
Hi Pankaj and Paolo,
Can we Queue this to merge list since there no other comments last 2 weeks?
Regards
Yi.
Quoted 42 lines · Zhang Yi
For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these
dev_daxorfs_daxpages to kvm for DIMM/NVDIMM backend, since these pages are reserved. the check ofkvm_is_reserved_pfn()misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to indentify these pages are from NVDIMM pmem. and let kvm treat these as normal pages.Without this patch, Many operations will be missed due to this mistreatment to pmem pages. For example, a page may not have chance to be unpinned for KVM
guest(in kvm_release_pfn_clean); not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.V1:
https://lkml.org/lkml/2018/7/4/91V2:
https://lkml.org/lkml/2018/7/10/135V3:
https://lkml.org/lkml/2018/8/9/17V4:
https://lkml.org/lkml/2018/8/22/17V5:
[PATCH V3 1/4] Reviewed-by: David / Acked-by: Pankaj [PATCH V3 2/4] Reviewed-by: Jan
[PATCH V3 3/4] Acked-by: Jan
[PATCH V3 4/4] Added "Acked-by: Pankaj", Added in-line comments: DaveZhang Yi (4):
kvm: remove redundant reserved page check
mm: introduce memory typeMEMORY_DEVICE_DEV_DAX
mm: add a function to differentiate the pages is from DAX device
memory
kvm: add a check if pfn is from NVDIMM pmem.
Quoted 9 lines
drivers/dax/pmem.c | 1 + include/linux/memremap.h | 8 ++++++++ include/linux/mm.h | 12 ++++++++++++ virt/kvm/kvm_main.c | 24 ++++++++++++++++-------- 4 files changed, 37 insertions(+), 8 deletions(-)Signature · 1 line
2.7.4
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Hello Yi,
Quoted 3 lines
Any comments?
Hi Pankaj and Paolo,
I am just helping with the review. Paolo & Dan probably will decide.
Thanks,
Pankaj
Quoted 60 lines
Can we Queue this to merge list since there no other comments last 2 weeks?
Regards
Yi.Quoted 51 lines · Zhang Yi
For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.
Now, we map these
dev_daxorfs_daxpages to kvm for DIMM/NVDIMM backend, since these pages are reserved. the check ofkvm_is_reserved_pfn()misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to indentify these pages are from NVDIMM pmem. and let kvm treat these as normal pages.Without this patch, Many operations will be missed due to this mistreatment to pmem pages. For example, a page may not have chance to be unpinned for KVM
guest(in kvm_release_pfn_clean); not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.V1:
https://lkml.org/lkml/2018/7/4/91V2:
https://lkml.org/lkml/2018/7/10/135V3:
https://lkml.org/lkml/2018/8/9/17V4:
https://lkml.org/lkml/2018/8/22/17V5:
[PATCH V3 1/4] Reviewed-by: David / Acked-by: Pankaj [PATCH V3 2/4] Reviewed-by: Jan
[PATCH V3 3/4] Acked-by: Jan
[PATCH V3 4/4] Added "Acked-by: Pankaj", Added in-line comments: DaveZhang Yi (4):
kvm: remove redundant reserved page check
mm: introduce memory typeMEMORY_DEVICE_DEV_DAX
mm: add a function to differentiate the pages is from DAX device
memory
kvm: add a check if pfn is from NVDIMM pmem.drivers/dax/pmem.c | 1 + include/linux/memremap.h | 8 ++++++++ include/linux/mm.h | 12 ++++++++++++ virt/kvm/kvm_main.c | 24 ++++++++++++++++-------- 4 files changed, 37 insertions(+), 8 deletions(-)--
2.7.4