diff mbox series

[RFC,6/6] WORKAROUND: Don't split large folios on madvise

Message ID 20230317105802.2634004-7-ryan.roberts@arm.com (mailing list archive)
State New
Headers show
Series variable-order, large folios for anonymous memory | expand

Commit Message

Ryan Roberts March 17, 2023, 10:58 a.m. UTC
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 mm/madvise.c | 8 ++++++++
 1 file changed, 8 insertions(+)

--
2.25.1

Comments

Yin, Fengwei March 22, 2023, 8:19 a.m. UTC | #1
On 3/17/23 18:58, Ryan Roberts wrote:
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>   mm/madvise.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 340125d08c03..8fb84da744e1 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -447,6 +447,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>   		 * are sure it's worth. Split it if we are only owner.
>   		 */
>   		if (folio_test_large(folio)) {
> +#if 0
>   			if (folio_mapcount(folio) != 1)
>   				break;
>   			if (pageout_anon_only_filter && !folio_test_anon(folio))
> @@ -469,6 +470,9 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>   			pte--;
>   			addr -= PAGE_SIZE;
>   			continue;
> +#else
> +			break;
> +#endif
>   		}
> 
>   		/*
> @@ -664,6 +668,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
>   		 * deactivate all pages.
>   		 */
>   		if (folio_test_large(folio)) {
> +#if 0
>   			if (folio_mapcount(folio) != 1)
>   				goto out;
>   			folio_get(folio);
> @@ -684,6 +689,9 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
>   			pte--;
>   			addr -= PAGE_SIZE;
>   			continue;
> +#else
> +			goto out;
> +#endif
>   		}
 From this workaround change, you hit an case that large folio has
1 as folio_mapcount()? Can you share the kernel crash log to me? Thanks.


Regards
Yin, Fengwei

> 
>   		if (folio_test_swapcache(folio) || folio_test_dirty(folio)) {
> --
> 2.25.1
>
Ryan Roberts March 22, 2023, 8:59 a.m. UTC | #2
On 22/03/2023 08:19, Yin Fengwei wrote:
> On 3/17/23 18:58, Ryan Roberts wrote:
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
>> ---
>>   mm/madvise.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/mm/madvise.c b/mm/madvise.c
>> index 340125d08c03..8fb84da744e1 100644
>> --- a/mm/madvise.c
>> +++ b/mm/madvise.c
>> @@ -447,6 +447,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>>            * are sure it's worth. Split it if we are only owner.
>>            */
>>           if (folio_test_large(folio)) {
>> +#if 0
>>               if (folio_mapcount(folio) != 1)
>>                   break;
>>               if (pageout_anon_only_filter && !folio_test_anon(folio))
>> @@ -469,6 +470,9 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>>               pte--;
>>               addr -= PAGE_SIZE;
>>               continue;
>> +#else
>> +            break;
>> +#endif
>>           }
>>
>>           /*
>> @@ -664,6 +668,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned
>> long addr,
>>            * deactivate all pages.
>>            */
>>           if (folio_test_large(folio)) {
>> +#if 0
>>               if (folio_mapcount(folio) != 1)
>>                   goto out;
>>               folio_get(folio);
>> @@ -684,6 +689,9 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned
>> long addr,
>>               pte--;
>>               addr -= PAGE_SIZE;
>>               continue;
>> +#else
>> +            goto out;
>> +#endif
>>           }
> From this workaround change, you hit an case that large folio has
> 1 as folio_mapcount()? Can you share the kernel crash log to me? Thanks.

Yes I do. I'm not sure why the mapcount is decreasing. I thought perhaps it
could be due to CoW or explicit munmap, or something like that. I've been trying
to find the reason that the mapcount is being reduced by using the page_ref
tracepoints, but its proving difficult.

The crash logs are somewhat random. But I'll send some to you separately.

Thanks for taking a look!

> 
> 
> Regards
> Yin, Fengwei
> 
>>
>>           if (folio_test_swapcache(folio) || folio_test_dirty(folio)) {
>> -- 
>> 2.25.1
>>
>
diff mbox series

Patch

diff --git a/mm/madvise.c b/mm/madvise.c
index 340125d08c03..8fb84da744e1 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -447,6 +447,7 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 		 * are sure it's worth. Split it if we are only owner.
 		 */
 		if (folio_test_large(folio)) {
+#if 0
 			if (folio_mapcount(folio) != 1)
 				break;
 			if (pageout_anon_only_filter && !folio_test_anon(folio))
@@ -469,6 +470,9 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 			pte--;
 			addr -= PAGE_SIZE;
 			continue;
+#else
+			break;
+#endif
 		}

 		/*
@@ -664,6 +668,7 @@  static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
 		 * deactivate all pages.
 		 */
 		if (folio_test_large(folio)) {
+#if 0
 			if (folio_mapcount(folio) != 1)
 				goto out;
 			folio_get(folio);
@@ -684,6 +689,9 @@  static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
 			pte--;
 			addr -= PAGE_SIZE;
 			continue;
+#else
+			goto out;
+#endif
 		}

 		if (folio_test_swapcache(folio) || folio_test_dirty(folio)) {