Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Thu, Mar 21, 2019 at 7:57 AM Dan Williams wrote: > > On Wed, Mar 20, 2019 at 8:34 AM Dan Williams wrote: > > > > On Wed, Mar 20, 2019 at 1:09 AM Aneesh Kumar K.V > > wrote: > > > > > > Aneesh Kumar K.V writes: > > > > > > > Dan Williams writes: > > > > > > > >> > > > >>> Now what will be page size used for mapping vmemmap? > > > >> > > > >> That's up to the architecture's vmemmap_populate() implementation. > > > >> > > > >>> Architectures > > > >>> possibly will use PMD_SIZE mapping if supported for vmemmap. Now a > > > >>> device-dax with struct page in the device will have pfn reserve area > > > >>> aligned > > > >>> to PAGE_SIZE with the above example? We can't map that using > > > >>> PMD_SIZE page size? > > > >> > > > >> IIUC, that's a different alignment. Currently that's handled by > > > >> padding the reservation area up to a section (128MB on x86) boundary, > > > >> but I'm working on patches to allow sub-section sized ranges to be > > > >> mapped. > > > > > > > > I am missing something w.r.t code. The below code align that using > > > > nd_pfn->align > > > > > > > > if (nd_pfn->mode == PFN_MODE_PMEM) { > > > > unsigned long memmap_size; > > > > > > > > /* > > > >* vmemmap_populate_hugepages() allocates the memmap > > > > array in > > > >* HPAGE_SIZE chunks. > > > >*/ > > > > memmap_size = ALIGN(64 * npfns, HPAGE_SIZE); > > > > offset = ALIGN(start + SZ_8K + memmap_size + > > > > dax_label_reserve, > > > > nd_pfn->align) - start; > > > > } > > > > > > > > IIUC that is finding the offset where to put vmemmap start. And that has > > > > to be aligned to the page size with which we may end up mapping vmemmap > > > > area right? > > > > Right, that's the physical offset of where the vmemmap ends, and the > > memory to be mapped begins. > > > > > > Yes we find the npfns by aligning up using PAGES_PER_SECTION. But that > > > > is to compute howmany pfns we should map for this pfn dev right? > > > > > > > > > > Also i guess those 4K assumptions there is wrong? > > > > Yes, I think to support non-4K-PAGE_SIZE systems the 'pfn' metadata > > needs to be revved and the PAGE_SIZE needs to be recorded in the > > info-block. > > How often does a system change page-size. Is it fixed or do > environment change it from one boot to the next? I'm thinking through > the behavior of what do when the recorded PAGE_SIZE in the info-block > does not match the current system page size. The simplest option is to > just fail the device and require it to be reconfigured. Is that > acceptable? The kernel page size is set at build time and as far as I know every distro configures their ppc64(le) kernel for 64K. I've used 4K kernels a few times in the past to debug PAGE_SIZE dependent problems, but I'd be surprised if anyone is using 4K in production. Anyway, my view is that using 4K here isn't really a problem since it's just the accounting unit of the pfn superblock format. The kernel reading form it should understand that and scale it to whatever accounting unit it wants to use internally. Currently we don't so that should probably be fixed, but that doesn't seem to cause any real issues. As far as I can tell the only user of npfns in __nvdimm_setup_pfn() whih prints the "number of pfns truncated" message. Am I missing something? > ___ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 20, 2019 at 8:34 AM Dan Williams wrote: > > On Wed, Mar 20, 2019 at 1:09 AM Aneesh Kumar K.V > wrote: > > > > Aneesh Kumar K.V writes: > > > > > Dan Williams writes: > > > > > >> > > >>> Now what will be page size used for mapping vmemmap? > > >> > > >> That's up to the architecture's vmemmap_populate() implementation. > > >> > > >>> Architectures > > >>> possibly will use PMD_SIZE mapping if supported for vmemmap. Now a > > >>> device-dax with struct page in the device will have pfn reserve area > > >>> aligned > > >>> to PAGE_SIZE with the above example? We can't map that using > > >>> PMD_SIZE page size? > > >> > > >> IIUC, that's a different alignment. Currently that's handled by > > >> padding the reservation area up to a section (128MB on x86) boundary, > > >> but I'm working on patches to allow sub-section sized ranges to be > > >> mapped. > > > > > > I am missing something w.r.t code. The below code align that using > > > nd_pfn->align > > > > > > if (nd_pfn->mode == PFN_MODE_PMEM) { > > > unsigned long memmap_size; > > > > > > /* > > >* vmemmap_populate_hugepages() allocates the memmap array > > > in > > >* HPAGE_SIZE chunks. > > >*/ > > > memmap_size = ALIGN(64 * npfns, HPAGE_SIZE); > > > offset = ALIGN(start + SZ_8K + memmap_size + > > > dax_label_reserve, > > > nd_pfn->align) - start; > > > } > > > > > > IIUC that is finding the offset where to put vmemmap start. And that has > > > to be aligned to the page size with which we may end up mapping vmemmap > > > area right? > > Right, that's the physical offset of where the vmemmap ends, and the > memory to be mapped begins. > > > > Yes we find the npfns by aligning up using PAGES_PER_SECTION. But that > > > is to compute howmany pfns we should map for this pfn dev right? > > > > > > > Also i guess those 4K assumptions there is wrong? > > Yes, I think to support non-4K-PAGE_SIZE systems the 'pfn' metadata > needs to be revved and the PAGE_SIZE needs to be recorded in the > info-block. How often does a system change page-size. Is it fixed or do environment change it from one boot to the next? I'm thinking through the behavior of what do when the recorded PAGE_SIZE in the info-block does not match the current system page size. The simplest option is to just fail the device and require it to be reconfigured. Is that acceptable? ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 20, 2019 at 1:09 AM Aneesh Kumar K.V wrote: > > Aneesh Kumar K.V writes: > > > Dan Williams writes: > > > >> > >>> Now what will be page size used for mapping vmemmap? > >> > >> That's up to the architecture's vmemmap_populate() implementation. > >> > >>> Architectures > >>> possibly will use PMD_SIZE mapping if supported for vmemmap. Now a > >>> device-dax with struct page in the device will have pfn reserve area > >>> aligned > >>> to PAGE_SIZE with the above example? We can't map that using > >>> PMD_SIZE page size? > >> > >> IIUC, that's a different alignment. Currently that's handled by > >> padding the reservation area up to a section (128MB on x86) boundary, > >> but I'm working on patches to allow sub-section sized ranges to be > >> mapped. > > > > I am missing something w.r.t code. The below code align that using > > nd_pfn->align > > > > if (nd_pfn->mode == PFN_MODE_PMEM) { > > unsigned long memmap_size; > > > > /* > >* vmemmap_populate_hugepages() allocates the memmap array in > >* HPAGE_SIZE chunks. > >*/ > > memmap_size = ALIGN(64 * npfns, HPAGE_SIZE); > > offset = ALIGN(start + SZ_8K + memmap_size + > > dax_label_reserve, > > nd_pfn->align) - start; > > } > > > > IIUC that is finding the offset where to put vmemmap start. And that has > > to be aligned to the page size with which we may end up mapping vmemmap > > area right? Right, that's the physical offset of where the vmemmap ends, and the memory to be mapped begins. > > Yes we find the npfns by aligning up using PAGES_PER_SECTION. But that > > is to compute howmany pfns we should map for this pfn dev right? > > > > Also i guess those 4K assumptions there is wrong? Yes, I think to support non-4K-PAGE_SIZE systems the 'pfn' metadata needs to be revved and the PAGE_SIZE needs to be recorded in the info-block. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
Aneesh Kumar K.V writes: > Dan Williams writes: > >> >>> Now what will be page size used for mapping vmemmap? >> >> That's up to the architecture's vmemmap_populate() implementation. >> >>> Architectures >>> possibly will use PMD_SIZE mapping if supported for vmemmap. Now a >>> device-dax with struct page in the device will have pfn reserve area aligned >>> to PAGE_SIZE with the above example? We can't map that using >>> PMD_SIZE page size? >> >> IIUC, that's a different alignment. Currently that's handled by >> padding the reservation area up to a section (128MB on x86) boundary, >> but I'm working on patches to allow sub-section sized ranges to be >> mapped. > > I am missing something w.r.t code. The below code align that using > nd_pfn->align > > if (nd_pfn->mode == PFN_MODE_PMEM) { > unsigned long memmap_size; > > /* >* vmemmap_populate_hugepages() allocates the memmap array in >* HPAGE_SIZE chunks. >*/ > memmap_size = ALIGN(64 * npfns, HPAGE_SIZE); > offset = ALIGN(start + SZ_8K + memmap_size + dax_label_reserve, > nd_pfn->align) - start; > } > > IIUC that is finding the offset where to put vmemmap start. And that has > to be aligned to the page size with which we may end up mapping vmemmap > area right? > > Yes we find the npfns by aligning up using PAGES_PER_SECTION. But that > is to compute howmany pfns we should map for this pfn dev right? > Also i guess those 4K assumptions there is wrong? modified drivers/nvdimm/pfn_devs.c @@ -783,7 +783,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) return -ENXIO; } - npfns = (size - offset - start_pad - end_trunc) / SZ_4K; + npfns = (size - offset - start_pad - end_trunc) / PAGE_SIZE; pfn_sb->mode = cpu_to_le32(nd_pfn->mode); pfn_sb->dataoff = cpu_to_le64(offset); pfn_sb->npfns = cpu_to_le64(npfns); -aneesh ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
Dan Williams writes: > >> Now what will be page size used for mapping vmemmap? > > That's up to the architecture's vmemmap_populate() implementation. > >> Architectures >> possibly will use PMD_SIZE mapping if supported for vmemmap. Now a >> device-dax with struct page in the device will have pfn reserve area aligned >> to PAGE_SIZE with the above example? We can't map that using >> PMD_SIZE page size? > > IIUC, that's a different alignment. Currently that's handled by > padding the reservation area up to a section (128MB on x86) boundary, > but I'm working on patches to allow sub-section sized ranges to be > mapped. I am missing something w.r.t code. The below code align that using nd_pfn->align if (nd_pfn->mode == PFN_MODE_PMEM) { unsigned long memmap_size; /* * vmemmap_populate_hugepages() allocates the memmap array in * HPAGE_SIZE chunks. */ memmap_size = ALIGN(64 * npfns, HPAGE_SIZE); offset = ALIGN(start + SZ_8K + memmap_size + dax_label_reserve, nd_pfn->align) - start; } IIUC that is finding the offset where to put vmemmap start. And that has to be aligned to the page size with which we may end up mapping vmemmap area right? Yes we find the npfns by aligning up using PAGES_PER_SECTION. But that is to compute howmany pfns we should map for this pfn dev right? -aneesh ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Tue, Mar 19, 2019 at 1:45 AM Kirill A. Shutemov wrote: > > On Wed, Mar 13, 2019 at 09:07:13AM -0700, Dan Williams wrote: > > On Wed, Mar 6, 2019 at 4:46 AM Aneesh Kumar K.V > > wrote: > > > > > > On 3/6/19 5:14 PM, Michal Suchánek wrote: > > > > On Wed, 06 Mar 2019 14:47:33 +0530 > > > > "Aneesh Kumar K.V" wrote: > > > > > > > >> Dan Williams writes: > > > >> > > > >>> On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > > > > > > On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > > > wrote: > > > > > > > >> Also even if the user decided to not use THP, by > > > >> echo "never" > transparent_hugepage/enabled , we should continue to map > > > >> dax fault using huge page on platforms that can support huge pages. > > > > > > > > Is this a good idea? > > > > > > > > This knob is there for a reason. In some situations having huge pages > > > > can severely impact performance of the system (due to host-guest > > > > interaction or whatever) and the ability to really turn off all THP > > > > would be important in those cases, right? > > > > > > > > > > My understanding was that is not true for dax pages? These are not > > > regular memory that got allocated. They are allocated out of /dev/dax/ > > > or /dev/pmem*. Do we have a reason not to use hugepages for mapping > > > pages in that case? > > > > The problem with the transparent_hugepage/enabled interface is that it > > conflates performing compaction work to produce THP-pages with the > > ability to map huge pages at all. > > That's not [entirely] true. transparent_hugepage/defrag gates heavy-duty > compaction. We do only very limited compaction if it's not advised by > transparent_hugepage/defrag. > > I believe DAX has to respect transparent_hugepage/enabled. Or not > advertise its huge pages as THP. It's confusing for user. What does "advertise its huge pages as THP" mean in practice? I think it's confusing that DAX, a facility that bypasses System RAM, is affected by a transparent_hugepage flag which is a feature for combining System RAM pages into larger pages. For the same reason that transparent_hugepage does not gate / control hugetlb operation is the same reason that transparent_hugepage should not gate / control DAX. A global setting to disable opportunistic large page mappings of System-RAM makes sense, but I don't see why that should read on DAX? ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 13, 2019 at 09:07:13AM -0700, Dan Williams wrote: > On Wed, Mar 6, 2019 at 4:46 AM Aneesh Kumar K.V > wrote: > > > > On 3/6/19 5:14 PM, Michal Suchánek wrote: > > > On Wed, 06 Mar 2019 14:47:33 +0530 > > > "Aneesh Kumar K.V" wrote: > > > > > >> Dan Williams writes: > > >> > > >>> On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > > > > On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > > wrote: > > > > > >> Also even if the user decided to not use THP, by > > >> echo "never" > transparent_hugepage/enabled , we should continue to map > > >> dax fault using huge page on platforms that can support huge pages. > > > > > > Is this a good idea? > > > > > > This knob is there for a reason. In some situations having huge pages > > > can severely impact performance of the system (due to host-guest > > > interaction or whatever) and the ability to really turn off all THP > > > would be important in those cases, right? > > > > > > > My understanding was that is not true for dax pages? These are not > > regular memory that got allocated. They are allocated out of /dev/dax/ > > or /dev/pmem*. Do we have a reason not to use hugepages for mapping > > pages in that case? > > The problem with the transparent_hugepage/enabled interface is that it > conflates performing compaction work to produce THP-pages with the > ability to map huge pages at all. That's not [entirely] true. transparent_hugepage/defrag gates heavy-duty compaction. We do only very limited compaction if it's not advised by transparent_hugepage/defrag. I believe DAX has to respect transparent_hugepage/enabled. Or not advertise its huge pages as THP. It's confusing for user. -- Kirill A. Shutemov ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 13, 2019 at 8:45 PM Aneesh Kumar K.V wrote: [..] > >> Now w.r.t to failures, can device-dax do an opportunistic huge page > >> usage? > > > > device-dax explicitly disclaims the ability to do opportunistic mappings. > > > >> I haven't looked at the device-dax details fully yet. Do we make the > >> assumption of the mapping page size as a format w.r.t device-dax? Is that > >> derived from nd_pfn->align value? > > > > Correct. > > > >> > >> Here is what I am working on: > >> 1) If the platform doesn't support huge page and if the device superblock > >> indicated that it was created with huge page support, we fail the device > >> init. > > > > Ok. > > > >> 2) Now if we are creating a new namespace without huge page support in > >> the platform, then we force the align details to PAGE_SIZE. In such a > >> configuration when handling dax fault even with THP enabled during > >> the build, we should not try to use hugepage. This I think we can > >> achieve by using TRANSPARENT_HUGEPAEG_DAX_FLAG. > > > > How is this dynamic property communicated to the guest? > > via device tree on powerpc. We have a device tree node indicating > supported page sizes. Ah, ok, yeah let's plumb that straight to the device-dax driver and leave out the interaction / interpretation of the thp-enabled flags. > > > > >> > >> Also even if the user decided to not use THP, by > >> echo "never" > transparent_hugepage/enabled , we should continue to map > >> dax fault using huge page on platforms that can support huge pages. > >> > >> This still doesn't cover the details of a device-dax created with > >> PAGE_SIZE align later booted with a kernel that can do hugepage dax.How > >> should we handle that? That makes me think, this should be a VMA flag > >> which got derived from device config? May be use VM_HUGEPAGE to indicate > >> if device should use a hugepage mapping or not? > > > > device-dax configured with PAGE_SIZE always gets PAGE_SIZE mappings. > > Now what will be page size used for mapping vmemmap? That's up to the architecture's vmemmap_populate() implementation. > Architectures > possibly will use PMD_SIZE mapping if supported for vmemmap. Now a > device-dax with struct page in the device will have pfn reserve area aligned > to PAGE_SIZE with the above example? We can't map that using > PMD_SIZE page size? IIUC, that's a different alignment. Currently that's handled by padding the reservation area up to a section (128MB on x86) boundary, but I'm working on patches to allow sub-section sized ranges to be mapped. Now, that said, I expect there may be bugs lurking in the implementation if PAGE_SIZE changes from one boot to the next simply because I've never tested that. I think this also indicates that the section padding logic can't be removed until all arch vmemmap_populate() implementations understand the sub-section case. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
Dan Williams writes: > On Wed, Mar 6, 2019 at 1:18 AM Aneesh Kumar K.V > wrote: >> >> Dan Williams writes: >> >> > On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: >> >> >> >> On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V >> >> wrote: >> >> > >> >> > Add a flag to indicate the ability to do huge page dax mapping. On >> >> > architecture >> >> > like ppc64, the hypervisor can disable huge page support in the guest. >> >> > In >> >> > such a case, we should not enable huge page dax mapping. This patch adds >> >> > a flag which the architecture code will update to indicate huge page >> >> > dax mapping support. >> >> >> >> *groan* >> >> >> >> > Architectures mostly do transparent_hugepage_flag = 0; if they can't >> >> > do hugepages. That also takes care of disabling dax hugepage mapping >> >> > with this change. >> >> > >> >> > Without this patch we get the below error with kvm on ppc64. >> >> > >> >> > [ 118.849975] lpar: Failed hash pte insert with error -4 >> >> > >> >> > NOTE: The patch also use >> >> > >> >> > echo never > /sys/kernel/mm/transparent_hugepage/enabled >> >> > to disable dax huge page mapping. >> >> > >> >> > Signed-off-by: Aneesh Kumar K.V >> >> > --- >> >> > TODO: >> >> > * Add Fixes: tag >> >> > >> >> > include/linux/huge_mm.h | 4 +++- >> >> > mm/huge_memory.c| 4 >> >> > 2 files changed, 7 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >> >> > index 381e872bfde0..01ad5258545e 100644 >> >> > --- a/include/linux/huge_mm.h >> >> > +++ b/include/linux/huge_mm.h >> >> > @@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct >> >> > *vma, unsigned long addr, >> >> > pud_t *pud, pfn_t pfn, bool write); >> >> > enum transparent_hugepage_flag { >> >> > TRANSPARENT_HUGEPAGE_FLAG, >> >> > + TRANSPARENT_HUGEPAGE_DAX_FLAG, >> >> > TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, >> >> > TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, >> >> > TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, >> >> > @@ -111,7 +112,8 @@ static inline bool >> >> > __transparent_hugepage_enabled(struct vm_area_struct *vma) >> >> > if (transparent_hugepage_flags & (1 << >> >> > TRANSPARENT_HUGEPAGE_FLAG)) >> >> > return true; >> >> > >> >> > - if (vma_is_dax(vma)) >> >> > + if (vma_is_dax(vma) && >> >> > + (transparent_hugepage_flags & (1 << >> >> > TRANSPARENT_HUGEPAGE_DAX_FLAG))) >> >> > return true; >> >> >> >> Forcing PTE sized faults should be fine for fsdax, but it'll break >> >> devdax. The devdax driver requires the fault size be >= the namespace >> >> alignment since devdax tries to guarantee hugepage mappings will be >> >> used and PMD alignment is the default. We can probably have devdax >> >> fall back to the largest size the hypervisor has made available, but >> >> it does run contrary to the design. Ah well, I suppose it's better off >> >> being degraded rather than unusable. >> > >> > Given this is an explicit setting I think device-dax should explicitly >> > fail to enable in the presence of this flag to preserve the >> > application visible behavior. >> > >> > I.e. if device-dax was enabled after this setting was made then I >> > think future faults should fail as well. >> >> Not sure I understood that. Now we are disabling the ability to map >> pages as huge pages. I am now considering that this should not be >> user configurable. Ie, this is something that platform can use to avoid >> dax forcing huge page mapping, but if the architecture can enable huge >> dax mapping, we should always default to using that. > > No, that's an application visible behavior regression. The side effect > of this setting is that all huge-page configured device-dax instances > must be disabled. So if the device was created with a nd_pfn->align value of PMD_SIZE, that is an indication that we would map the pages in PMD_SIZE? Ok with that understanding, If the align value is not a supported mapping size, we fail initializing the device. > >> Now w.r.t to failures, can device-dax do an opportunistic huge page >> usage? > > device-dax explicitly disclaims the ability to do opportunistic mappings. > >> I haven't looked at the device-dax details fully yet. Do we make the >> assumption of the mapping page size as a format w.r.t device-dax? Is that >> derived from nd_pfn->align value? > > Correct. > >> >> Here is what I am working on: >> 1) If the platform doesn't support huge page and if the device superblock >> indicated that it was created with huge page support, we fail the device >> init. > > Ok. > >> 2) Now if we are creating a new namespace without huge page support in >> the platform, then we force the align details to PAGE_SIZE. In such a >> configuration when handling dax fault even with THP enabled during >> the build, we should not try to use hugepage. This I think we can >> achieve by using
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 6, 2019 at 4:46 AM Aneesh Kumar K.V wrote: > > On 3/6/19 5:14 PM, Michal Suchánek wrote: > > On Wed, 06 Mar 2019 14:47:33 +0530 > > "Aneesh Kumar K.V" wrote: > > > >> Dan Williams writes: > >> > >>> On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > > On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > wrote: > > > >> Also even if the user decided to not use THP, by > >> echo "never" > transparent_hugepage/enabled , we should continue to map > >> dax fault using huge page on platforms that can support huge pages. > > > > Is this a good idea? > > > > This knob is there for a reason. In some situations having huge pages > > can severely impact performance of the system (due to host-guest > > interaction or whatever) and the ability to really turn off all THP > > would be important in those cases, right? > > > > My understanding was that is not true for dax pages? These are not > regular memory that got allocated. They are allocated out of /dev/dax/ > or /dev/pmem*. Do we have a reason not to use hugepages for mapping > pages in that case? The problem with the transparent_hugepage/enabled interface is that it conflates performing compaction work to produce THP-pages with the ability to map huge pages at all. The compaction is a nop for dax because the memory is already statically allocated. If the administrator does not want dax to consume huge TLB entries then don't configure huge-page dax. If a hypervisor wants to force disable huge-page-configured device-dax instances after the fact it seems we need an explicit interface for that and not overload transparent_hugepage/enabled. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 6, 2019 at 1:18 AM Aneesh Kumar K.V wrote: > > Dan Williams writes: > > > On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > >> > >> On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > >> wrote: > >> > > >> > Add a flag to indicate the ability to do huge page dax mapping. On > >> > architecture > >> > like ppc64, the hypervisor can disable huge page support in the guest. In > >> > such a case, we should not enable huge page dax mapping. This patch adds > >> > a flag which the architecture code will update to indicate huge page > >> > dax mapping support. > >> > >> *groan* > >> > >> > Architectures mostly do transparent_hugepage_flag = 0; if they can't > >> > do hugepages. That also takes care of disabling dax hugepage mapping > >> > with this change. > >> > > >> > Without this patch we get the below error with kvm on ppc64. > >> > > >> > [ 118.849975] lpar: Failed hash pte insert with error -4 > >> > > >> > NOTE: The patch also use > >> > > >> > echo never > /sys/kernel/mm/transparent_hugepage/enabled > >> > to disable dax huge page mapping. > >> > > >> > Signed-off-by: Aneesh Kumar K.V > >> > --- > >> > TODO: > >> > * Add Fixes: tag > >> > > >> > include/linux/huge_mm.h | 4 +++- > >> > mm/huge_memory.c| 4 > >> > 2 files changed, 7 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > >> > index 381e872bfde0..01ad5258545e 100644 > >> > --- a/include/linux/huge_mm.h > >> > +++ b/include/linux/huge_mm.h > >> > @@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct > >> > *vma, unsigned long addr, > >> > pud_t *pud, pfn_t pfn, bool write); > >> > enum transparent_hugepage_flag { > >> > TRANSPARENT_HUGEPAGE_FLAG, > >> > + TRANSPARENT_HUGEPAGE_DAX_FLAG, > >> > TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, > >> > TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, > >> > TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, > >> > @@ -111,7 +112,8 @@ static inline bool > >> > __transparent_hugepage_enabled(struct vm_area_struct *vma) > >> > if (transparent_hugepage_flags & (1 << > >> > TRANSPARENT_HUGEPAGE_FLAG)) > >> > return true; > >> > > >> > - if (vma_is_dax(vma)) > >> > + if (vma_is_dax(vma) && > >> > + (transparent_hugepage_flags & (1 << > >> > TRANSPARENT_HUGEPAGE_DAX_FLAG))) > >> > return true; > >> > >> Forcing PTE sized faults should be fine for fsdax, but it'll break > >> devdax. The devdax driver requires the fault size be >= the namespace > >> alignment since devdax tries to guarantee hugepage mappings will be > >> used and PMD alignment is the default. We can probably have devdax > >> fall back to the largest size the hypervisor has made available, but > >> it does run contrary to the design. Ah well, I suppose it's better off > >> being degraded rather than unusable. > > > > Given this is an explicit setting I think device-dax should explicitly > > fail to enable in the presence of this flag to preserve the > > application visible behavior. > > > > I.e. if device-dax was enabled after this setting was made then I > > think future faults should fail as well. > > Not sure I understood that. Now we are disabling the ability to map > pages as huge pages. I am now considering that this should not be > user configurable. Ie, this is something that platform can use to avoid > dax forcing huge page mapping, but if the architecture can enable huge > dax mapping, we should always default to using that. No, that's an application visible behavior regression. The side effect of this setting is that all huge-page configured device-dax instances must be disabled. > Now w.r.t to failures, can device-dax do an opportunistic huge page > usage? device-dax explicitly disclaims the ability to do opportunistic mappings. > I haven't looked at the device-dax details fully yet. Do we make the > assumption of the mapping page size as a format w.r.t device-dax? Is that > derived from nd_pfn->align value? Correct. > > Here is what I am working on: > 1) If the platform doesn't support huge page and if the device superblock > indicated that it was created with huge page support, we fail the device > init. Ok. > 2) Now if we are creating a new namespace without huge page support in > the platform, then we force the align details to PAGE_SIZE. In such a > configuration when handling dax fault even with THP enabled during > the build, we should not try to use hugepage. This I think we can > achieve by using TRANSPARENT_HUGEPAEG_DAX_FLAG. How is this dynamic property communicated to the guest? > > Also even if the user decided to not use THP, by > echo "never" > transparent_hugepage/enabled , we should continue to map > dax fault using huge page on platforms that can support huge pages. > > This still doesn't cover the details of a device-dax created with > PAGE_SIZE align later booted with a kernel that can do hugepage dax.How
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On 3/6/19 5:14 PM, Michal Suchánek wrote: On Wed, 06 Mar 2019 14:47:33 +0530 "Aneesh Kumar K.V" wrote: Dan Williams writes: On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V wrote: Also even if the user decided to not use THP, by echo "never" > transparent_hugepage/enabled , we should continue to map dax fault using huge page on platforms that can support huge pages. Is this a good idea? This knob is there for a reason. In some situations having huge pages can severely impact performance of the system (due to host-guest interaction or whatever) and the ability to really turn off all THP would be important in those cases, right? My understanding was that is not true for dax pages? These are not regular memory that got allocated. They are allocated out of /dev/dax/ or /dev/pmem*. Do we have a reason not to use hugepages for mapping pages in that case? -aneesh ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, Mar 06, 2019 at 06:15:25PM +0530, Aneesh Kumar K.V wrote: > On 3/6/19 5:14 PM, Michal Suchánek wrote: > > On Wed, 06 Mar 2019 14:47:33 +0530 > > "Aneesh Kumar K.V" wrote: > > > > > Dan Williams writes: > > > > > > > On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > > > > > > > > > > On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > > > > > wrote: > > > Also even if the user decided to not use THP, by > > > echo "never" > transparent_hugepage/enabled , we should continue to map > > > dax fault using huge page on platforms that can support huge pages. > > > > Is this a good idea? > > > > This knob is there for a reason. In some situations having huge pages > > can severely impact performance of the system (due to host-guest > > interaction or whatever) and the ability to really turn off all THP > > would be important in those cases, right? > > > > My understanding was that is not true for dax pages? These are not regular > memory that got allocated. They are allocated out of /dev/dax/ or > /dev/pmem*. Do we have a reason not to use hugepages for mapping pages in > that case? Yes. Like when you don't want dax to compete for TLB with mission-critical application (which uses hugetlb for instance). -- Kirill A. Shutemov ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
On Wed, 06 Mar 2019 14:47:33 +0530 "Aneesh Kumar K.V" wrote: > Dan Williams writes: > > > On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: > >> > >> On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V > >> wrote: > Also even if the user decided to not use THP, by > echo "never" > transparent_hugepage/enabled , we should continue to map > dax fault using huge page on platforms that can support huge pages. Is this a good idea? This knob is there for a reason. In some situations having huge pages can severely impact performance of the system (due to host-guest interaction or whatever) and the ability to really turn off all THP would be important in those cases, right? Thanks Michal ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 2/2] mm/dax: Don't enable huge dax mapping by default
Dan Williams writes: > On Thu, Feb 28, 2019 at 1:40 AM Oliver wrote: >> >> On Thu, Feb 28, 2019 at 7:35 PM Aneesh Kumar K.V >> wrote: >> > >> > Add a flag to indicate the ability to do huge page dax mapping. On >> > architecture >> > like ppc64, the hypervisor can disable huge page support in the guest. In >> > such a case, we should not enable huge page dax mapping. This patch adds >> > a flag which the architecture code will update to indicate huge page >> > dax mapping support. >> >> *groan* >> >> > Architectures mostly do transparent_hugepage_flag = 0; if they can't >> > do hugepages. That also takes care of disabling dax hugepage mapping >> > with this change. >> > >> > Without this patch we get the below error with kvm on ppc64. >> > >> > [ 118.849975] lpar: Failed hash pte insert with error -4 >> > >> > NOTE: The patch also use >> > >> > echo never > /sys/kernel/mm/transparent_hugepage/enabled >> > to disable dax huge page mapping. >> > >> > Signed-off-by: Aneesh Kumar K.V >> > --- >> > TODO: >> > * Add Fixes: tag >> > >> > include/linux/huge_mm.h | 4 +++- >> > mm/huge_memory.c| 4 >> > 2 files changed, 7 insertions(+), 1 deletion(-) >> > >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >> > index 381e872bfde0..01ad5258545e 100644 >> > --- a/include/linux/huge_mm.h >> > +++ b/include/linux/huge_mm.h >> > @@ -53,6 +53,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_area_struct >> > *vma, unsigned long addr, >> > pud_t *pud, pfn_t pfn, bool write); >> > enum transparent_hugepage_flag { >> > TRANSPARENT_HUGEPAGE_FLAG, >> > + TRANSPARENT_HUGEPAGE_DAX_FLAG, >> > TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, >> > TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, >> > TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, >> > @@ -111,7 +112,8 @@ static inline bool >> > __transparent_hugepage_enabled(struct vm_area_struct *vma) >> > if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG)) >> > return true; >> > >> > - if (vma_is_dax(vma)) >> > + if (vma_is_dax(vma) && >> > + (transparent_hugepage_flags & (1 << >> > TRANSPARENT_HUGEPAGE_DAX_FLAG))) >> > return true; >> >> Forcing PTE sized faults should be fine for fsdax, but it'll break >> devdax. The devdax driver requires the fault size be >= the namespace >> alignment since devdax tries to guarantee hugepage mappings will be >> used and PMD alignment is the default. We can probably have devdax >> fall back to the largest size the hypervisor has made available, but >> it does run contrary to the design. Ah well, I suppose it's better off >> being degraded rather than unusable. > > Given this is an explicit setting I think device-dax should explicitly > fail to enable in the presence of this flag to preserve the > application visible behavior. > > I.e. if device-dax was enabled after this setting was made then I > think future faults should fail as well. Not sure I understood that. Now we are disabling the ability to map pages as huge pages. I am now considering that this should not be user configurable. Ie, this is something that platform can use to avoid dax forcing huge page mapping, but if the architecture can enable huge dax mapping, we should always default to using that. Now w.r.t to failures, can device-dax do an opportunistic huge page usage? I haven't looked at the device-dax details fully yet. Do we make the assumption of the mapping page size as a format w.r.t device-dax? Is that derived from nd_pfn->align value? Here is what I am working on: 1) If the platform doesn't support huge page and if the device superblock indicated that it was created with huge page support, we fail the device init. 2) Now if we are creating a new namespace without huge page support in the platform, then we force the align details to PAGE_SIZE. In such a configuration when handling dax fault even with THP enabled during the build, we should not try to use hugepage. This I think we can achieve by using TRANSPARENT_HUGEPAEG_DAX_FLAG. Also even if the user decided to not use THP, by echo "never" > transparent_hugepage/enabled , we should continue to map dax fault using huge page on platforms that can support huge pages. This still doesn't cover the details of a device-dax created with PAGE_SIZE align later booted with a kernel that can do hugepage dax.How should we handle that? That makes me think, this should be a VMA flag which got derived from device config? May be use VM_HUGEPAGE to indicate if device should use a hugepage mapping or not? -aneesh ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm