Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <05bc90b6-4954-b945-f0d8-373f565c1248@intel.com>
Date:   Wed, 26 Jul 2023 10:49:49 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.0
Subject: Re: [RFC PATCH v2 2/4] madvise: Use notify-able API to clear and
 flush page table entries
Content-Language: en-US
To:     Yu Zhao <yuzhao@google.com>
CC:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        <akpm@linux-foundation.org>, <minchan@kernel.org>,
        <willy@infradead.org>, <david@redhat.com>, <ryan.roberts@arm.com>,
        <shy828301@gmail.com>
References: <20230721094043.2506691-1-fengwei.yin@intel.com>
 <20230721094043.2506691-3-fengwei.yin@intel.com>
 <CAOUHufY2tdO0JNTiY=RzHitR7CB1cM5kA=7bd6nbCUW6KM_OVA@mail.gmail.com>
From:   Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <CAOUHufY2tdO0JNTiY=RzHitR7CB1cM5kA=7bd6nbCUW6KM_OVA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eDJ2YlgwY1U5UzEvYlk4SXVSSG9VWGNudXB4L3FoYWVEQ2V4eGV2Sk84TDM2?=
 =?utf-8?B?d0tSbWdWcVROZ0FJV1BEaURLaW92ZlE0UVNFa0UwRU1xVjdvTEZVdkRqRENW?=
 =?utf-8?B?WTN1a0NmZ2JiZHd2Y0VGRWdPdDc1N08vK2RGd1hoVEllamhGL1JDRWo2eTZj?=
 =?utf-8?B?MXJQRHlaNFpPbmsvUDZnekhjN3ZLbVNOTHp1YzRySmliT2d0cjc3bzcwaTlq?=
 =?utf-8?B?M3ByOWVxRElRdUZJdFAvOTNvb0tIMExVN0lQcG9QZzM0T0NJdVV6ci9DQWdv?=
 =?utf-8?B?eFY4Y2lpUC9mWks4NjQzVHptSE9iRm5YblRWd1FGeUYxcUlwaDZVdWFZMm5G?=
 =?utf-8?B?RnpsWnZvRWR4YnRMNlJvVjdDVU02cm1Wdkk2Rkp3YmxEL1VWV0U3WkRZU2tj?=
 =?utf-8?B?bWc3d0lLYnVZVHU1eGhHWVB1ZzNyWHZ6VGUvWGtDdDBGM2MvWXp6azNCOEpp?=
 =?utf-8?B?M05vWVN0dGpVQmxIdEREQWNWNlY4ME5tbFpYeTJvL05xMHVmdVBndU43N094?=
 =?utf-8?B?anliY1pDbTB1SkNPbGpJMkxLS0VJcGRCSURUb0szQ01aSjhyY0dKQWRsbVVp?=
 =?utf-8?B?c21Ed1NwZHAzMzlqSVprU3hyak8xanlidlozR3JuU0NETllpaE1TY1JnMUpZ?=
 =?utf-8?B?YUU5eVNaRWw1YmZEelVLOFp1emRxVnZjOWpDRG9pT1BGOEpaWU80U01JaEZU?=
 =?utf-8?B?MFJiUytoWkZXQVN0eFhoOW01QXJGMkJSRHYxZFZUcFk0VVNhckQrNEpLYms4?=
 =?utf-8?B?Z3J0R0pxZTZpRU4ycUtvV0pUVW9RanIzV0RKdk04RjNpTmVWVE9RNzJKd3RP?=
 =?utf-8?B?K0thOXlQWU96MnZlY29wTkZvUmprUWtqRG1aN09PdDYzVlRRZUl0Mk5EL3dF?=
 =?utf-8?B?aWtFdDN4ZDFSOTM3OXVLbEJrWjJ1dDJUTmkvMnFJekZYeG5Fak5yU1JmZm92?=
 =?utf-8?B?bHBNYXV2Ulp2cVlkS1hWM2twMGlrWHpOUU1pYXdtYkRSZ0syVjB0N3JRWk0y?=
 =?utf-8?B?czFvVmV1QTdrdkJFYnRjWGp3NHF5Wmw0MERxZEpaTzZlMUlyVWQyVS8zNlo1?=
 =?utf-8?B?NXRqMDVtRk83ODhXZ1dvQ3BSRVJDUmJIa2grbzRlY0l3bHlySGxDc3l0dXpv?=
 =?utf-8?B?UlZ3Y1lwVTE3aVJ6cDV3ekluVW1YK1RnSnVYNU1hVjlTalBhREhvUkhGVHZo?=
 =?utf-8?B?ak9JSjVoYXVpSXZZR2pKZ3NaTW8rMEpjbU9IenJwcmp3VGVBbzJ5aGpkUnNq?=
 =?utf-8?B?VTlDcWVlcVN4czFUOGhEcnJCcktSd3pUM0NBTktzT0owdWliTU5naFByUWt6?=
 =?utf-8?B?S2RHTytRMUx5QXJSMGo3V3hKSGE0THNGVEZLbVdxbHpXcXpzQ0NiYkMvVm5I?=
 =?utf-8?B?QmFlN1R1eWx4Qks3STR5L3hrZzV2SVZGNUl3aXRxeElKMjRkSFczODJFNTdM?=
 =?utf-8?B?TndQM0VDM01GQk4yalpaMjlXbnlGQm12YVBtOXVBbythZytzc2RhZHZEbXhq?=
 =?utf-8?B?V1VxWmx3cDVMOVVsSE1Da1M4cGhwaEFaWDB4b2ticGVZZmJWVTdId3d3V3M0?=
 =?utf-8?B?aHUvNFd5ZXBYZlBRV3lLSXI0blgvYjMrNzkrQ2hRd1pQWHBWTGRVZ3hJZ1Zy?=
 =?utf-8?B?MFF3MXBQcmREK3NJRnVQeWlwZUtmenZnQ2x5SGgzZGgwQWQzTjE4QzZ0TFdm?=
 =?utf-8?B?ZGQySzdHb0ZuTG9JM2NBK215UGo4SlVpVm4rRkxJTW5tSGtBZFN0M2xTRHo4?=
 =?utf-8?B?YTdYVVdKTHViUUppcEhEaXBpMkdhNmlwNjhDZ0tLaUo0VHRmL3o2MkpsQ0Y3?=
 =?utf-8?B?c3E1RzRIei9HQjFseENMK0d1a2JENjMrS2lxUWFRZjA5TDFlTWQ2bWp1Q095?=
 =?utf-8?B?NTk1UUZ6Qlp3UzZPcU5jWE1Xc1hTTzBINDZ0ZDhBRHdWeEw1K2w0Z1FlK0dQ?=
 =?utf-8?B?QWJ1T0hPTzQ3M3hpcGdxTk9Od1BJQ0dacE5rT1pTa3RGSkppU3NqU210REs2?=
 =?utf-8?B?ckU0S3VLbVVnMG5JVFhtaGRqdkNpQi9acmJISTNIUW16NkRkKzF3QUdkRkNh?=
 =?utf-8?B?SGMySEhlN3E1cyt5ZkMvVHJOQjY5MTVFOHAwTjk0d3NQTmQ3UlFxdUx3cElC?=
 =?utf-8?B?dElRMnQvVGZCckF6bFVGZldpSjk5RUg2eDZnMG9DSFROMXV6SXdidTI0TEpD?=
 =?utf-8?B?UXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ffacb418-f518-4c76-cb8d-08db8d82f77e
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jul 2023 02:49:44.1590
 (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: 6qMC+N/51Z7PAYLbkgXhyeCBQESPDELLxs2Ws8zHlulR0LvKDJfVlzociOdJ8sxGyrh82TMAz8nue6byS/nemg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6496
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/25/23 13:55, Yu Zhao wrote:
> On Fri, Jul 21, 2023 at 3:41 AM Yin Fengwei <fengwei.yin@intel.com> wrote:
>>
>> Currently, in function madvise_cold_or_pageout_pte_range(), the
>> young bit of pte/pmd is cleared notify subscripter.
>>
>> Using notify-able API to make sure the subscripter is signaled about
>> the young bit clearing.
>>
>> Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
>> ---
>>  mm/madvise.c | 18 ++----------------
>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/madvise.c b/mm/madvise.c
>> index f12933ebcc24..b236e201a738 100644
>> --- a/mm/madvise.c
>> +++ b/mm/madvise.c
>> @@ -403,14 +403,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>>                         return 0;
>>                 }
>>
>> -               if (pmd_young(orig_pmd)) {
>> -                       pmdp_invalidate(vma, addr, pmd);
>> -                       orig_pmd = pmd_mkold(orig_pmd);
>> -
>> -                       set_pmd_at(mm, addr, pmd, orig_pmd);
>> -                       tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
>> -               }
>> -
>> +               pmdp_clear_flush_young_notify(vma, addr, pmd);
>>                 folio_clear_referenced(folio);
>>                 folio_test_clear_young(folio);
>>                 if (folio_test_active(folio))
>> @@ -496,14 +489,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>>
>>                 VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
>>
>> -               if (pte_young(ptent)) {
>> -                       ptent = ptep_get_and_clear_full(mm, addr, pte,
>> -                                                       tlb->fullmm);
>> -                       ptent = pte_mkold(ptent);
>> -                       set_pte_at(mm, addr, pte, ptent);
>> -                       tlb_remove_tlb_entry(tlb, pte, addr);
>> -               }
>> -
>> +               ptep_clear_flush_young_notify(vma, addr, pte);
> 
> These two places are tricky.
> 
> I agree there is a problem here, i.e., we are not consulting the mmu
> notifier. In fact, we do pageout on VMs on ChromeOS, and it's been a
> known problem to me for a while (not a high priority one).
> 
> tlb_remove_tlb_entry() is batched flush, ptep_clear_flush_young() is
> not. But, on x86, we might see a performance improvement since
> ptep_clear_flush_young() doesn't flush TLB at all. On ARM, there might
> be regressions though.
> 
> I'd go with ptep_clear_young_notify(), but IIRC, Minchan mentioned he
> prefers flush. So I'll let him chime in.
I am OK with either way even no flush way here is more efficient for
arm64. Let's wait for Minchan's comment.

> 
> If we do end up with ptep_clear_young_notify(), please remove
> mmu_gather -- it should have been done in this patch.

I suppose "remove mmu_gather" means to trigger flush tlb operation in
batched way to make sure no stale data in TLB for long time on arm64
platform.


Regards
Yin, Fengwei