Received-SPF: pass (google.com: domain of linux-kernel+bounces-82566-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <19758162-be5f-4dc4-b316-77b0115d12ce@intel.com>
Date: Tue, 27 Feb 2024 09:51:13 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/1] mm/madvise: enhance lazyfreeing with mTHP in
 madvise_free
Content-Language: en-US
To: Barry Song <21cnbao@gmail.com>, Ryan Roberts <ryan.roberts@arm.com>
CC: Lance Yang <ioworker0@gmail.com>, David Hildenbrand <david@redhat.com>,
	<akpm@linux-foundation.org>, <linux-kernel@vger.kernel.org>,
	<linux-mm@kvack.org>, <mhocko@suse.com>, <minchan@kernel.org>,
	<peterx@redhat.com>, <shy828301@gmail.com>, <songmuchun@bytedance.com>,
	<wangkefeng.wang@huawei.com>, <zokeefe@google.com>
References: <CAGsJ_4zera4+bWuXHKjdU3QdiR3sqcUQB3gF08DPD49OCT4S9w@mail.gmail.com>
 <20240226083714.26187-1-ioworker0@gmail.com>
 <9bcf5141-7376-441e-bbe3-779956ef28b9@redhat.com>
 <CAK1f24mdwjz2J5VmeYSDwb4jMbrXUVCSF_0pOcWSVt1cfa0FhQ@mail.gmail.com>
 <318be511-06de-423e-8216-af869f27f849@arm.com>
 <CAGsJ_4z+0yXUDYwxKNAqYwxAAYpfKpKd2u_dVTDP3b-KPOQT1g@mail.gmail.com>
From: Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <CAGsJ_4z+0yXUDYwxKNAqYwxAAYpfKpKd2u_dVTDP3b-KPOQT1g@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dVpSa3RBamlDdldUSzBFUVQrODc5MWJEd1hIUzFjNE5UYlBRQ1hqNEx3UjBw?=
 =?utf-8?B?NGFaVXFWQmZKZklzaS85c1M4Ri9qQnA0WDB1ZmhCQkdUQ25vcHdHalp6YjQr?=
 =?utf-8?B?SDJhMWFSOVZtTnNUY0pWRGxvdXhpMGpzQ1plc1RBcDArOVV1d29hZS9tcGs4?=
 =?utf-8?B?Sk5TYXY1QlFvU05ySXR2YTZYNTNsb2FrZTBSSUhLdWdzOEVhZm9LckRSZEhq?=
 =?utf-8?B?c2xsNG9KTXBuWFNnQ1c0Z3FuNEtZbnNoVUpzclNsSDZZYTV5Nzk0VmRSTDBD?=
 =?utf-8?B?d05qWndSVnhqTW9IQjNwTUgva094THRnenFkc1lwaWZvZmkwSThvRTBtQVRE?=
 =?utf-8?B?UDkrL0tRSkhXdWpJVFAxTG1IWDBmQmYzazN1cmJ0ckdnZHhDZnU0QmFRM2JB?=
 =?utf-8?B?SzV1MDZrelR5QTdhNGY5Smt6RCtvU3hMUWpYaGV6VlJHTWgvaVVtQ0dRWnRN?=
 =?utf-8?B?Yk1uekpTVXdienBKSTFnY2Zyei9BK1FYZnlISWluZklvRGhvTk42NjR1eTBZ?=
 =?utf-8?B?VXhGYUhMSUJYNlM2Z2k2TjRFY3FSZFhTQVp5aGdWaTl6QjRXMkpVaXJUVHda?=
 =?utf-8?B?R2cveSs2ZWN6RGNhUnZPQTFpbi8xUVBES2JQY3pzbi9FNGJGS0JBdTVNUG5W?=
 =?utf-8?B?L2pWdzQzNE1vTitBemRhYW1FNTFhNGZCdWgwRERwLzdpTnpKZldvZmlKUHVt?=
 =?utf-8?B?WmM2UHRrcks5Z2s4V0wxK1pTNHNsblhKWFdCK2s0aE94anZDaml0NlhvT0FQ?=
 =?utf-8?B?V1FuUitMeVI3NXVWUVJ5am1DcGNqNnFucEpXZjRhdTRuVVdtQW5EYTZVanUw?=
 =?utf-8?B?YWQ0d2Q1UlJyck5PcTkzT2ltMnRiWnBCNUdMWFdyTElxeVpRbHdBKzNtcUow?=
 =?utf-8?B?Z1RaeDVLY045a0hnZnZJUzFnbUNwUkRFbVVYRDFDN2hFcFAxQU9FdFJtMUVi?=
 =?utf-8?B?WG5jS3FONGsrNUhGODNNYzZhbXcyUDliQ3p3RGY4d3pxeDZQbEhJcFErN3hM?=
 =?utf-8?B?cmxqdXRZcTBYeUNuV3dJSS9IS1J2SW1RQ3BWVHRDTTBPV2RESUlCdThDbVBn?=
 =?utf-8?B?NGxRMERNVkZ0aHBSQVUzZGtybDdxUlRBTURxUE5LQ0FDZmQ4blJoVFdQanhC?=
 =?utf-8?B?SHlLVWcxQklGMkFzYjV6cWxjOEViRGNsWGdxMm9kbHBvRFhUYlB5RmVOQ1dC?=
 =?utf-8?B?aTd3WEhtQngvclpjN2huU09jZ3BscXF4Zms2K0lpdnAyVWlIUHJkSW5qQkN6?=
 =?utf-8?B?RlhhTlZNTlFFcW56OFFWMUJSUmJMTlVKZDJicGlqeXpGRkNvTXFiRTJHd3lw?=
 =?utf-8?B?c1hwaUVGNWxHdkdTUE02bTQ5aUttSFVqRk1UZHZIV2x2M3hIZWtsOUhZMTVC?=
 =?utf-8?B?b2VsYnlIK0cwZWFyTlVBN1FCWjhYaGhKa3BURVhTSm9XRHBYSkl3MlE1dE9w?=
 =?utf-8?B?eEh0bTMzRTFzMmkvWXV3b0J6dnljWldRZ1FvNmdzaGFiMnBlRXBJRllqc3hF?=
 =?utf-8?B?NXVWdFcvUWVLZ09VaTQwSUQ0OFVaaVh3K3gvUkFCeHcwQ25NZStZcE5SSVNo?=
 =?utf-8?B?Z0J0ZmJ5b0hCOFRwV2UyL0RiK3BVcHhRVDhIc3UyMmFuWjltWnl6VFZyVVBo?=
 =?utf-8?B?eFg5clZTOXBsK2xuY3JGdDVtdDZjYkUrbE9JRjNZQWx1RXc1UFVpQ2x3VjRF?=
 =?utf-8?B?Y1FZUWo2dFFpd3cvVS8wdzBKeXY4Qkg5aHJBTDVieEhxL25ZYkJ1U1BDN1k3?=
 =?utf-8?B?S1BDZDdkT3ozbFV2ZkQ5c1VaN2tRUTU0WERIV2VhZFN5bzMrcWUrRnFVeUN1?=
 =?utf-8?B?QUlpdk80bjIyck43eFhwUk9SMUdiNlgzZVB0SElhTFdQMTdqbXpPcjJ3N2lh?=
 =?utf-8?B?aThnUkpXYy9RZDVTcEpuSGQ3Q24xL0JkWlllcFNxQjFxd0JUS2JUU1dEUzdZ?=
 =?utf-8?B?enFIMWNCbTd2UjlrSUFDei83N2Z1eDA5RlJXdTgzb0ZvbTJyakdUWE81emNT?=
 =?utf-8?B?YmtRM3NhK0xSMXo1aEt0UDBLMHJHK21DWXczRDJWMGVCK2tONXJUdlVyb3BQ?=
 =?utf-8?B?RjRUUTZPaWFiQmRmNm1YWlMwcTVHeVhjNUowajBiMDlMQnZ5WktFRGVoTVpN?=
 =?utf-8?Q?fMp1/2gLu1JmjoUJW1v+8pWoP?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e9220ffd-120e-4d86-f41f-08dc3736991f
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Feb 2024 01:51:21.7769
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GUpOu/d0NQq2K6c9zGgyq4BU1/av7UoLDUDUdkRhw6qWVrrtPw3kvHYS37pbwnSWnG1uMoBMV1nlGmGFVh/JNA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6186
X-OriginatorOrg: intel.com


On 2/27/24 04:49, Barry Song wrote:
> On Tue, Feb 27, 2024 at 2:04 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 26/02/2024 08:55, Lance Yang wrote:
>>> Hey David,
>>>
>>> Thanks for your suggestion!
>>>
>>> On Mon, Feb 26, 2024 at 4:41 PM David Hildenbrand <david@redhat.com> wrote:
>>>>
>>> [...]
>>>>> On Mon, Feb 26, 2024 at 12:00 PM Barry Song <21cnbao@gmail.com> wrote:
>>>>> [...]
>>>>>> On Mon, Feb 26, 2024 at 1:33 AM Lance Yang <ioworker0@gmail.com> wrote:
>>>>> [...]
>>> [...]
>>>>> +static inline bool pte_range_cont_mapped(pte_t *pte, unsigned long nr)
>>>>> +{
>>>>> +     pte_t pte_val;
>>>>> +     unsigned long pfn = pte_pfn(pte);
>>>>> +     for (int i = 0; i < nr; i++) {
>>>>> +             pte_val = ptep_get(pte + i);
>>>>> +             if (pte_none(pte_val) || pte_pfn(pte_val) != (pfn + i))
>>>>> +                     return false;
>>>>> +     }
>>>>> +     return true;
>>>>> +}
>>>>
>>>> I dislike the "cont mapped" terminology.
>>>>
>>>> Maybe folio_pte_batch() does what you want?
>>>
>>> folio_pte_batch() is a good choice. Appreciate it!
>>
>> Agreed, folio_pte_batch() is likely to be widely useful for this change and
>> others, so suggest exporting it from memory.c and reusing as is if possible.
> 
> I actually missed folio_pte_batch() in cont-pte series and re-invented
> a function
> to check if a large folio is entirely mapped in MADV_PAGEOUT[1]. exporting
> folio_pte_batch() will also benefit that case. The problem space is same.
> 
> [1] https://lore.kernel.org/linux-mm/20240118111036.72641-7-21cnbao@gmail.com/
I am wondering whether we can delay large folio split till page reclaim phase
for madvise cases.

Like if we hit folio which is partially mapped to the range, don't split it but
just unmap the mapping part from the range. Let page reclaim decide whether
split the large folio or not (If it's not mapped to any other range,it will be
freed as whole large folio. If part of it still mapped to other range,page reclaim
can decide whether to split it or ignore it for current reclaim cycle).

Splitting does work here. But it just drops all the benefits of large folio.


Regards
Yin, Fengwei

> 
>>
>>>
>>> Best,
>>> Lance
>>>
>>>>
>>>> --
>>>> Cheers,
>>>>
>>>> David / dhildenb
> 
> Thanks
> Barry