Messages in this thread | | | Date | Wed, 16 Aug 2023 17:11:54 +0200 | Subject | Re: [PATCH v2 3/3] madvise:madvise_free_pte_range(): don't use mapcount() against large folio for sharing check | From | David Hildenbrand <> |
| |
On 16.08.23 16:13, Daniel Gomez wrote: > On Wed, Aug 16, 2023 at 08:04:11PM +0800, Yin, Fengwei wrote: >> >> >> On 8/16/2023 7:44 PM, Daniel Gomez wrote: >>> On Wed, Aug 16, 2023 at 07:30:35AM +0800, Yin Fengwei wrote: >>>> >>>> >>>> On 8/15/23 21:25, Daniel Gomez wrote: >>>>> Hi Yin, >>>>> On Tue, Aug 08, 2023 at 10:09:17AM +0800, Yin Fengwei wrote: >>>>>> Commit 98b211d6415f ("madvise: convert madvise_free_pte_range() to use a >>>>>> folio") replaced the page_mapcount() with folio_mapcount() to check >>>>>> whether the folio is shared by other mapping. >>>>>> >>>>>> It's not correct for large folios. folio_mapcount() returns the total >>>>>> mapcount of large folio which is not suitable to detect whether the folio >>>>>> is shared. >>>>>> >>>>>> Use folio_estimated_sharers() which returns a estimated number of shares. >>>>>> That means it's not 100% correct. It should be OK for madvise case here. >>>>> >>>>> I'm trying to understand why it should be ok for madvise this change, so >>>>> I hope it's okay to ask you few questions. >>>>> >>>>> folio_mapcount() calculates the total maps for all the subpages of a >>>>> folio. However, the folio_estimated_sharers does it only for the first >>>>> subpage making it not true for large folios. Then, wouldn't this change >>>>> drop support for large folios? >>>> I saw David explained this very well in another mail. >>>> >>>>> >>>>> Seems like folio_entire_mapcount() is not accurate either because of it >>>>> does not inclue PTE-mapped sub-pages which I think we need here. Hence, >>>>> the folio_mapcount(). Could this be something missing in the test side? >>>>> >>>>> I tried to replicate the setup with CONFIG_TRANSPARENT_HUGEPAGE but >>>>> seems like I'm not able to do it: >>>>> >>>>> ./cow >>>>> # [INFO] detected THP size: 2048 KiB >>>>> # [INFO] detected hugetlb size: 2048 KiB >>>>> # [INFO] detected hugetlb size: 1048576 KiB >>>>> # [INFO] huge zeropage is enabled >>>>> TAP version 13 >>>>> 1..166 >>>>> # [INFO] Anonymous memory tests in private mappings >>>>> # [RUN] Basic COW after fork() ... with base page >>>>> not ok 1 MADV_NOHUGEPAGE failed >>>>> # [RUN] Basic COW after fork() ... with swapped out base page >>>>> not ok 2 MADV_NOHUGEPAGE failed >>>>> # [RUN] Basic COW after fork() ... with THP >>>>> not ok 3 MADV_HUGEPAGE failed >>>>> # [RUN] Basic COW after fork() ... with swapped-out THP >>>>> not ok 4 MADV_HUGEPAGE failed >>>>> # [RUN] Basic COW after fork() ... with PTE-mapped THP >>>>> not ok 5 MADV_HUGEPAGE failed >>>>> # [RUN] Basic COW after fork() ... with swapped-out, PTE-mapped THP >>>>> not ok 6 MADV_HUGEPAGE failed >>>>> ... >>>> Can you post the MADV_PAGEOUT and PTE-mapped THP related testing result? >>>> And I suppose swap need be enabled also for the testing. >>> >>> You may find a dump of the logs in the link below with system information. Let me >>> know if you find something wrong in my setup or if you need something else. >>> Besides CONFIG_TRANSPARENT_HUGEPAGE, CONFIG_SWAP is also enabled in the kernel. >>> >>> https://gitlab.com/-/snippets/2584135 >>> >>> Also, strace reports ENOSYS for MADV_*: >>> madvise(0x7f2912465000, 4096, MADV_NOHUGEPAGE) = -1 ENOSYS (Function not implemented) >>> madvise(0x7f2912000000, 2097152, MADV_HUGEPAGE) = -1 ENOSYS (Function not implemented) >> O. The problem here is MADV_HUGEPAGE/MADV_NOHUGEPAGE doesn't work. >> Do you have CONFIG_ADVISE_SYSCALLS enabled? > It worked after I enabled the conf. Some tests failed and some were > skipped. But I managed to reproduce the issue now, thanks Yin! > > Bail out! 4 out of 166 tests failed > # Totals: pass:146 fail:4 xfail:0 xpass:0 skip:16 error:0 >
These hugetlb that are failing are known failures.
-- Cheers,
David / dhildenb
| |