Received-SPF: pass (google.com: domain of linux-kernel+bounces-46254-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <d7e1e877-80ad-48ce-b11e-2c60e951ec8b@intel.com>
Date: Wed, 31 Jan 2024 18:43:38 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 9/9] mm/memory: optimize unmap/zap with PTE-mapped THP
To: David Hildenbrand <david@redhat.com>, <linux-kernel@vger.kernel.org>
CC: <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>, "Matthew
 Wilcox" <willy@infradead.org>, Ryan Roberts <ryan.roberts@arm.com>, "Catalin
 Marinas" <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, "Aneesh
 Kumar K.V" <aneesh.kumar@linux.ibm.com>, Nick Piggin <npiggin@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>, Michael Ellerman <mpe@ellerman.id.au>,
	Christophe Leroy <christophe.leroy@csgroup.eu>, "Naveen N. Rao"
	<naveen.n.rao@linux.ibm.com>, Heiko Carstens <hca@linux.ibm.com>, "Vasily
 Gorbik" <gor@linux.ibm.com>, Alexander Gordeev <agordeev@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>, Sven Schnelle
	<svens@linux.ibm.com>, Arnd Bergmann <arnd@arndb.de>,
	<linux-arch@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>,
	<linux-s390@vger.kernel.org>
References: <20240129143221.263763-1-david@redhat.com>
 <20240129143221.263763-10-david@redhat.com>
 <2375481c-9d61-4f06-9f96-232f25b0e49b@intel.com>
 <d83309fa-4daa-430f-ae52-4e72162bca9a@redhat.com>
Content-Language: en-US
From: "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <d83309fa-4daa-430f-ae52-4e72162bca9a@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
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?T1BwcXkwYmNKOTMwM2Nzc040NGhyRTYyZEJtVkdTaTc3UDlqL1d4TU9oa3pD?=
 =?utf-8?B?N2VnUGh1ZzVWVnd2d1JDQTRIaUNzQ0tHSUh4Mysrb1hoYXcrOHJDa0RlcUlV?=
 =?utf-8?B?MVJrNmRQZFJQR2FkLzlCakR2bkh1K3M2RW9SYXZjUUQyQlZ6UFZGQmFGMWVC?=
 =?utf-8?B?c25RZmRhSTBRTGdDMitPZEIxM1A1WWlYNGx6M3lzM0ZQVklDUjUvVms5Zjdl?=
 =?utf-8?B?Tkk5SlhRZEdIY1JwSnF5RTFJeUhGQk5VSk45NUhleW13Qnd3clBmYXB0KytK?=
 =?utf-8?B?RGg0UjVoblpHYTZFZ01aZHcvSlZWSEtnamhDT0lpeHVPd0xmTDRHZnc5SlRl?=
 =?utf-8?B?U0I2YndFS01hR1FNSXIwL0RJa1V4U0JXNzZJa1o1NkE0TGlIM0ZSdlQ0UFRv?=
 =?utf-8?B?YnM5SWhMTThFUlhvR1JFV1BUL3dGcGZRalUzeXBnNkxhUWh0SjhxQUJOL3FK?=
 =?utf-8?B?Q0JXWUp2VzVGVUlrdDZTMDdLU2NBd0lhS2ZzSnZuOWJraXpEdHRWb0VmODNI?=
 =?utf-8?B?R1o3VXpCbGlmSytSRHB5TFhJTGEzS2toSVFQdFRPcDRDcWFvTDlpUFk2K0J5?=
 =?utf-8?B?MmNDU1lIcUtsVnArdFcrK2NjZ2dSMVVaQkxYaDhwWWNCbGNka3UrOUN4ZVdE?=
 =?utf-8?B?THRKREZ0SHZPYzF0a3pncXdwMXB5WkcvVUlKWXZ0RU5NY2dIWG9IZEp5RU5h?=
 =?utf-8?B?K1hBc2RMTWpLM0QwNlFta0M0SE9TY1FhdS9rWlkwZDBZaThBSGFMQXkwbG9S?=
 =?utf-8?B?eDV1U0NLM3hFeTQ0Y1c4VTJVTk8xN3JoUFJLTFRFTzdXZzlWMEk2QTcwWVBJ?=
 =?utf-8?B?Rk5LaC95dXJEWXZTaTNtWWxzU3RRZjdhZEliMHVFV0FJekxwbmxwUWwxTzhY?=
 =?utf-8?B?Y1Bjb253Q0NNdUJGS1kwUTJ5RW5QSWFNdjNTc0RUVzZpQklYSWJlbmQ0aFlO?=
 =?utf-8?B?dHJWYlZLVU94SjBzRzBxSCtKMHJYbytkN2RIUFo0azhWR2w2Wm5MbjZLVlUw?=
 =?utf-8?B?SDdoUWhEbWxtWEZjRlVJbHMvb2lWelM2TmN3S3hWZnBkT3F2dlBaeXg2MUFS?=
 =?utf-8?B?UkRhcFcreTRqZ1g5Z3B0MFFPOVhHalgyVnp3eXJYYzBJQUFhdTJFeVpHQVQ0?=
 =?utf-8?B?M000Z0w0RSt5UHN6ZWxjazZFSmd3Q0ltcWwweUY2SUR2RlJEc2hPZkZhbVVs?=
 =?utf-8?B?TXpYSnlDU1plRnk3MGNYTklZNHJlTU8zeWhHdTdTMU5taENaNS95NzdUY3pX?=
 =?utf-8?B?d25mclUzNXVIdHFsZ1pRc1dGRU9PQU9UVEx0Vmt6ZDY2NVZCbVJuWHNEZmdB?=
 =?utf-8?B?dEVoTWFPYm95U2U2ZjZsVFBuTWYzVWJTeFVhTnpzMCtUVElQNElzaWJiVEpE?=
 =?utf-8?B?c0w1bXdyNjhQY1RnRWY1V2pCUVJxVTlHMlh6T2oxSWhCTE8zMFB4SlJoNjFp?=
 =?utf-8?B?QWJ2aStkcUhwRVo4NXA5bDVVcEE4YVpKYjJhWkJIVGNwWGFYZnRrdWlvNDhO?=
 =?utf-8?B?TFcreExsU0NKdnppODZydHVsRXhkOVNRK2RZSThYMTJNZVRQNEM3UFNsemNP?=
 =?utf-8?B?eERnR05TOXVHRm94VUdoQzNLb3d5WFdFazkyWkg4ckRzbGRjRHJVQ0pScW9I?=
 =?utf-8?B?WDBsald4RUhxcENSMUFyZ2pHWGxOZVR6ekI4Nk9jRFdBWCs2U1liVDJtQTZh?=
 =?utf-8?B?VlZySDJURUM5Nm5SV0kwWmFDOVo0SThabDZodkRSc1d4ZittM20wRWx0dGhD?=
 =?utf-8?B?VDBxYlJwR2ZpVWhncVM4SHczYy9XZFEwTUN2L3NuaWVpR1IwSHNsVDZwMVpn?=
 =?utf-8?B?emdrR2xOWkh1Z3BmQlU1bWhUejNIQkVLdXA2eS9KUXFZaldGaVB3UnYwVy9P?=
 =?utf-8?B?TEcyV1NtZmYzUHl4R2F3VnhDWjJFSStGOTBzUTJNZDc5RFFNR1dVZWpaam44?=
 =?utf-8?B?TjdCUk9ISUdyenpwVDlTRGNoZm5VVDgxS1VOTDk4bHJHRFpTaHJPc1JadS9X?=
 =?utf-8?B?emp4NkorazNKTkVRdFhNbG1BSXpwOE5HWTZIaVhmc0M4dzRJSHFUbjVxck1W?=
 =?utf-8?B?c2x2MGQ0cmpSTlBWcXZNLzNsTmt3OEYxbXVrUlpWQ3pWZjdZT25oL1pCcitH?=
 =?utf-8?Q?FG140XjI5sBcIVYyo9LGgiftP?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9706f2de-c9ff-4c6b-32ed-08dc224981a3
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2024 10:43:48.5721
 (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: EtDIJe50qYmfE1/ZlNO+nXbnUoJOXLCzQ1MVygYor3/72vGAJhCYhlZ9zcdpFN1t6cwAiPibYzos5uShtpaFxA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6965
X-OriginatorOrg: intel.com


On 1/31/2024 6:30 PM, David Hildenbrand wrote:
> On 31.01.24 03:30, Yin Fengwei wrote:
>>
>>
>> On 1/29/24 22:32, David Hildenbrand wrote:
>>> +static inline pte_t get_and_clear_full_ptes(struct mm_struct *mm,
>>> +        unsigned long addr, pte_t *ptep, unsigned int nr, int full)
>>> +{
>>> +    pte_t pte, tmp_pte;
>>> +
>>> +    pte = ptep_get_and_clear_full(mm, addr, ptep, full);
>>> +    while (--nr) {
>>> +        ptep++;
>>> +        addr += PAGE_SIZE;
>>> +        tmp_pte = ptep_get_and_clear_full(mm, addr, ptep, full);
>>> +        if (pte_dirty(tmp_pte))
>>> +            pte = pte_mkdirty(pte);
>>> +        if (pte_young(tmp_pte))
>>> +            pte = pte_mkyoung(pte);
>> I am wondering whether it's worthy to move the pte_mkdirty() and 
>> pte_mkyoung()
>> out of the loop and just do it one time if needed. The worst case is 
>> that they
>> are called nr - 1 time. Or it's just too micro?
> 
> I also thought about just indicating "any_accessed" or "any_dirty" using 
> flags to the caller, to avoid the PTE modifications completely. Felt a 
> bit micro-optimized.
> 
> Regarding your proposal: I thought about that as well, but my assumption 
> was that dirty+young are "cheap" to be set.
> 
> On x86, pte_mkyoung() is setting _PAGE_ACCESSED.
> pte_mkdirty() is setting _PAGE_DIRTY | _PAGE_SOFT_DIRTY, but it also has 
> to handle the saveddirty handling, using some bit trickery.
> 
> So at least for pte_mkyoung() there would be no real benefit as far as I 
> can see (might be even worse). For pte_mkdirty() there might be a small 
> benefit.
> 
> Is it going to be measurable? Likely not.
Yeah. We can do more investigation when performance profiling call this
out.


Regards
Yin, Fengwei

> 
> Am I missing something?
> 
> Thanks!
>