| Message ID | 1565335998-22553-1-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm/debug: Add tests for architecture exported page table helpers | expand |
On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: > Should alloc_gigantic_page() be made available as an interface for general > use in the kernel. The test module here uses very similar implementation from > HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which > needs to be exported through a header. Why are you allocating memory at all instead of just using some known-to-exist PFNs like I suggested?
On 08/09/2019 03:46 PM, Matthew Wilcox wrote: > On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: >> Should alloc_gigantic_page() be made available as an interface for general >> use in the kernel. The test module here uses very similar implementation from >> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which >> needs to be exported through a header. > > Why are you allocating memory at all instead of just using some > known-to-exist PFNs like I suggested? We needed PFN to be PUD aligned for pfn_pud() and PMD aligned for mk_pmd(). Now walking the kernel page table for a known symbol like kernel_init() as you had suggested earlier we might encounter page table page entries at PMD and PUD which might not be PMD or PUD aligned respectively. It seemed to me that alignment requirement is applicable only for mk_pmd() and pfn_pud() which create large mappings at those levels but that requirement does not exist for page table pages pointing to next level. Is not that correct ? Or I am missing something here ?
On Fri, Aug 09, 2019 at 03:16:33AM -0700, Matthew Wilcox wrote: > On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: > > Should alloc_gigantic_page() be made available as an interface for general > > use in the kernel. The test module here uses very similar implementation from > > HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which > > needs to be exported through a header. > > Why are you allocating memory at all instead of just using some > known-to-exist PFNs like I suggested? IIUC the issue is that there aren't necessarily known-to-exist PFNs that are sufficiently aligned -- they may not even exist. For example, with 64K pages, a PMD covers 512M. The kernel image is (generally) smaller than 512M, and will be mapped at page granularity. In that case, any PMD entry for a kernel symbol address will point to the PTE level table, and that will only necessarily be page-aligned, as any P?D level table is only necessarily page-aligned. In the same configuration, you could have less than 512M of total memory, and none of this memory is necessarily aligned to 512M. So beyond the PTE level, I don't think you can guarantee a known-to-exist valid PFN. I also believe that synthetic PFNs could fail pfn_valid(), so that might cause us pain too... Thanks, Mark.
On Fri, Aug 09, 2019 at 04:05:07PM +0530, Anshuman Khandual wrote: > On 08/09/2019 03:46 PM, Matthew Wilcox wrote: > > On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: > >> Should alloc_gigantic_page() be made available as an interface for general > >> use in the kernel. The test module here uses very similar implementation from > >> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which > >> needs to be exported through a header. > > > > Why are you allocating memory at all instead of just using some > > known-to-exist PFNs like I suggested? > > We needed PFN to be PUD aligned for pfn_pud() and PMD aligned for mk_pmd(). > Now walking the kernel page table for a known symbol like kernel_init() I didn't say to walk the kernel page table. I said to call virt_to_pfn() for a known symbol like kernel_init(). > as you had suggested earlier we might encounter page table page entries at PMD > and PUD which might not be PMD or PUD aligned respectively. It seemed to me > that alignment requirement is applicable only for mk_pmd() and pfn_pud() > which create large mappings at those levels but that requirement does not > exist for page table pages pointing to next level. Is not that correct ? Or > I am missing something here ? Just clear the bottom bits off the PFN until you get a PMD or PUD aligned PFN. It's really not hard.
On 08/09/2019 05:14 PM, Mark Rutland wrote: > On Fri, Aug 09, 2019 at 03:16:33AM -0700, Matthew Wilcox wrote: >> On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: >>> Should alloc_gigantic_page() be made available as an interface for general >>> use in the kernel. The test module here uses very similar implementation from >>> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which >>> needs to be exported through a header. >> >> Why are you allocating memory at all instead of just using some >> known-to-exist PFNs like I suggested? > > IIUC the issue is that there aren't necessarily known-to-exist PFNs that > are sufficiently aligned -- they may not even exist. > > For example, with 64K pages, a PMD covers 512M. The kernel image is > (generally) smaller than 512M, and will be mapped at page granularity. > In that case, any PMD entry for a kernel symbol address will point to > the PTE level table, and that will only necessarily be page-aligned, as > any P?D level table is only necessarily page-aligned. Right. > > In the same configuration, you could have less than 512M of total > memory, and none of this memory is necessarily aligned to 512M. So > beyond the PTE level, I don't think you can guarantee a known-to-exist > valid PFN. Right a PMD aligned valid PFN might not even exist. This proposed patch which attempts to allocate memory chunk with required alignment will just fail indicating that such a valid PFN does not exist and hence will skip any relevant tests. At present this is done for PUD aligned allocation failure but we can similarly skip PMD relevant tests as well if PMD aligned memory chunk is not allocated. > > I also believe that synthetic PFNs could fail pfn_valid(), so that might > cause us pain too... Agreed. So do we have an agreement that it is better to use allocated memory with required alignment for the tests than known-to-exist PFNs ? - Anshuman
On 08/09/2019 07:22 PM, Matthew Wilcox wrote: > On Fri, Aug 09, 2019 at 04:05:07PM +0530, Anshuman Khandual wrote: >> On 08/09/2019 03:46 PM, Matthew Wilcox wrote: >>> On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: >>>> Should alloc_gigantic_page() be made available as an interface for general >>>> use in the kernel. The test module here uses very similar implementation from >>>> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which >>>> needs to be exported through a header. >>> >>> Why are you allocating memory at all instead of just using some >>> known-to-exist PFNs like I suggested? >> >> We needed PFN to be PUD aligned for pfn_pud() and PMD aligned for mk_pmd(). >> Now walking the kernel page table for a known symbol like kernel_init() > > I didn't say to walk the kernel page table. I said to call virt_to_pfn() > for a known symbol like kernel_init(). > >> as you had suggested earlier we might encounter page table page entries at PMD >> and PUD which might not be PMD or PUD aligned respectively. It seemed to me >> that alignment requirement is applicable only for mk_pmd() and pfn_pud() >> which create large mappings at those levels but that requirement does not >> exist for page table pages pointing to next level. Is not that correct ? Or >> I am missing something here ? > > Just clear the bottom bits off the PFN until you get a PMD or PUD aligned > PFN. It's really not hard. As Mark pointed out earlier that might end up being just a synthetic PFN which might not even exist on a given system.
On Mon, Aug 26, 2019 at 08:07:13AM +0530, Anshuman Khandual wrote: > On 08/09/2019 07:22 PM, Matthew Wilcox wrote: > > On Fri, Aug 09, 2019 at 04:05:07PM +0530, Anshuman Khandual wrote: > >> On 08/09/2019 03:46 PM, Matthew Wilcox wrote: > >>> On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: > >>>> Should alloc_gigantic_page() be made available as an interface for general > >>>> use in the kernel. The test module here uses very similar implementation from > >>>> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which > >>>> needs to be exported through a header. > >>> > >>> Why are you allocating memory at all instead of just using some > >>> known-to-exist PFNs like I suggested? > >> > >> We needed PFN to be PUD aligned for pfn_pud() and PMD aligned for mk_pmd(). > >> Now walking the kernel page table for a known symbol like kernel_init() > > > > I didn't say to walk the kernel page table. I said to call virt_to_pfn() > > for a known symbol like kernel_init(). > > > >> as you had suggested earlier we might encounter page table page entries at PMD > >> and PUD which might not be PMD or PUD aligned respectively. It seemed to me > >> that alignment requirement is applicable only for mk_pmd() and pfn_pud() > >> which create large mappings at those levels but that requirement does not > >> exist for page table pages pointing to next level. Is not that correct ? Or > >> I am missing something here ? > > > > Just clear the bottom bits off the PFN until you get a PMD or PUD aligned > > PFN. It's really not hard. > > As Mark pointed out earlier that might end up being just a synthetic PFN > which might not even exist on a given system. And why would that matter?
On 08/26/2019 06:43 PM, Matthew Wilcox wrote: > On Mon, Aug 26, 2019 at 08:07:13AM +0530, Anshuman Khandual wrote: >> On 08/09/2019 07:22 PM, Matthew Wilcox wrote: >>> On Fri, Aug 09, 2019 at 04:05:07PM +0530, Anshuman Khandual wrote: >>>> On 08/09/2019 03:46 PM, Matthew Wilcox wrote: >>>>> On Fri, Aug 09, 2019 at 01:03:17PM +0530, Anshuman Khandual wrote: >>>>>> Should alloc_gigantic_page() be made available as an interface for general >>>>>> use in the kernel. The test module here uses very similar implementation from >>>>>> HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which >>>>>> needs to be exported through a header. >>>>> >>>>> Why are you allocating memory at all instead of just using some >>>>> known-to-exist PFNs like I suggested? >>>> >>>> We needed PFN to be PUD aligned for pfn_pud() and PMD aligned for mk_pmd(). >>>> Now walking the kernel page table for a known symbol like kernel_init() >>> >>> I didn't say to walk the kernel page table. I said to call virt_to_pfn() >>> for a known symbol like kernel_init(). >>> >>>> as you had suggested earlier we might encounter page table page entries at PMD >>>> and PUD which might not be PMD or PUD aligned respectively. It seemed to me >>>> that alignment requirement is applicable only for mk_pmd() and pfn_pud() >>>> which create large mappings at those levels but that requirement does not >>>> exist for page table pages pointing to next level. Is not that correct ? Or >>>> I am missing something here ? >>> >>> Just clear the bottom bits off the PFN until you get a PMD or PUD aligned >>> PFN. It's really not hard. >> >> As Mark pointed out earlier that might end up being just a synthetic PFN >> which might not even exist on a given system. > > And why would that matter? > To start with the test uses struct page with mk_pte() and mk_pmd() while pfn gets used in pfn_pud() during pXX_basic_tests(). So we will not be able to derive a valid struct page from a synthetic pfn. Also if synthetic pfn is going to be used anyway then why derive it from a real kernel symbol like kernel_init(). Could not one be just made up with right alignment ? Currently the test allocates 'mm_struct' and other page table pages from real memory then why should it use synthetic pfn while creating actual page table entries ? Couple of benefits going with synthetic pfn will be.. - It simplifies the test a bit removing PUD_SIZE allocation helpers - It might enable the test to be run on systems without adequate memory In the current proposal the allocation happens during boot making it much more likely to succeed than not and when it fails, respective tests will be skipped. I am just wondering if being able to run complete set of tests on smaller systems with less memory weighs lot more in favor of going with synthetic pfn instead.