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: <7e09d69e-50a7-04c5-0a40-065e2d6fd658@intel.com>
Date:   Fri, 14 Jul 2023 13:57:26 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.13.0
Subject: Re: [RFC PATCH] madvise: make madvise_cold_or_pageout_pte_range()
 support large folio
Content-Language: en-US
To:     Yu Zhao <yuzhao@google.com>
CC:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        <akpm@linux-foundation.org>, <willy@infradead.org>,
        <david@redhat.com>, <ryan.roberts@arm.com>, <shy828301@gmail.com>
References: <20230713150558.200545-1-fengwei.yin@intel.com>
 <CAOUHufZBoV-ujMF3v60Sj4bARb5vRg7cpcRsqTiKHB4CKZc_fw@mail.gmail.com>
From:   "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <CAOUHufZBoV-ujMF3v60Sj4bARb5vRg7cpcRsqTiKHB4CKZc_fw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UGhqd250d1NYQnFaZ2tlYnQvRGJBQ1FWbE5OZ0JOZXcvYUt2UExDQnlBSlI4?=
 =?utf-8?B?RGpvUjRUK3RmVXBDUUlZRVJmc2FRWGlrMjlUbGFxZEQyWXVtOFB5SkJNekx5?=
 =?utf-8?B?U1RFaUlMN3pLaWY0L0kvS3IxOUhHcTkyd2xiUFFDR2tzVjg3L2pwWWR6b1h5?=
 =?utf-8?B?bWdWeTY2ZkVLQzRDUHdSY08xdHBwOE9ZV09UMWtBR3R2QjNISWRsSTVMUE8w?=
 =?utf-8?B?YnhxNUNrM1lIcGxRd0FvQjBHQVNGeHQ0QkhnTHNvQTVaUTc2WGJ4UHRGZlM4?=
 =?utf-8?B?Rm0yMmtORzdGcmRmaXQzT0piSVl4NEV3UXRhU3MrZTlPekpLOGJtWG9NZjdY?=
 =?utf-8?B?a0c0WG56UU9yWGtPVHZuWlJVbyt1dGxCRUtOZTh5V2VZL3FidTErNXRNbmdO?=
 =?utf-8?B?YzZFdEpicG5URUI1TnNzSEh4SS81SXAvWGtYWEU3YkdOZG1mOGFkOHdVeDdX?=
 =?utf-8?B?NTA3QUxtOG5XQzRnemk2RUFBa203MloyUDE4d2o1bkFRUG9BYkR5M1dEVHdJ?=
 =?utf-8?B?cDhTMVYybnpwRzlOOVZmUzEyS0VrRU51dHhQbzdGck1JNTBRZDFTQVVCZzRQ?=
 =?utf-8?B?MWQ4WS9ZZEJtaG1vYkYxWFpYSHZ3NDNMNzh4eTIyUkJ2KzhTQXhXZzZHcFdC?=
 =?utf-8?B?RXBxZ2ppWXk5SjJRTmh0TGhjUjUvUUNvclBvK2ZrZDVFdTZNeEJHMEVMNlpr?=
 =?utf-8?B?NGJCbWZtKzhSS1BkOThYY2dRYnEyTW5FbDUyOHk5K2NpTDNKUk1Dc2Rtajdn?=
 =?utf-8?B?VzYxS1RRZ0RQWGtGYml1MFB1NnZSNHZxMFpCcVNvcmVDT1FxVWtVMUc0aUlE?=
 =?utf-8?B?dk1mcjdjR1F0NGY5aHZGOU4yZ3pHSEdGVDhKUGkyUjdPcVJVZnNFcEJGVmo1?=
 =?utf-8?B?VmpBNUZTVFNYdEdOT2VhemplUzdhU20zRk9vc2FpT2dDWEpsZCtBaDZQY3Ja?=
 =?utf-8?B?bEw4RVkxKzVFY3hwbDZKSWNxZ3lzRUZkcVhoVTR5a0FCWE5RdVprMGRmMlVM?=
 =?utf-8?B?UGpQVCt4K2RudkZmcXpZL0hyVTVETWNOQjNGcnd3N0Q5T2piVzhzZGtsYTF1?=
 =?utf-8?B?R0EwNXpJRE1pNE9QMWhPbEYrbzhvRlF3RGRZUHhEZUxGR2lML0lzWDA3dHph?=
 =?utf-8?B?Tmt4bEh2bXoxanFuL2Z3M0k2OFZXWURMbS80MU5IZytCcFBKNFlzbENnNFlp?=
 =?utf-8?B?dWZzNk40MzBoN3RBMy9MUks5Y0lReHQ0RitUWEZmUFBKTzRPMllzNHZNZ2lT?=
 =?utf-8?B?eFFBMlM3eHNtVTZMY0N4YUtrcUtkZXc1UzVQblJBdTdYallsV2F4bUk0Zjgx?=
 =?utf-8?B?SDZMWUVUQ1NQVWh4VEFFYmM2SWZEYXprdTJqK1dHZDZZOGdsOWZmTTVOQ2hz?=
 =?utf-8?B?bHl2VGtLaWNObmNOOG4rT1FIelcvTDMwaGJlZlpNZEtGUmpiNWhlMzlBaU9u?=
 =?utf-8?B?cWFIU3BtdWd5L2loNFFEZUljNm5DaWxiTjNhU0hpZXN4UzQwNk51YjNvZmlW?=
 =?utf-8?B?Y1hZSGdzeXNuS05BQkdsckR0cDkrcnk4TTR6QVJsdFNvM1hEMDQ1bUJMLzVs?=
 =?utf-8?B?Vlc3MlRkV1JJTXp6VXZSb1I3TzV1V216WHRscjFia29hY0hZd3RuRVloRUhm?=
 =?utf-8?B?bHk2a3kwQ0tER0ZUaHR1NkxsbjdnbElwVENFdzIxMC9wWjY1L09SZFdIT0po?=
 =?utf-8?B?ZWhrcnEzc0tQVnNaWUVNSGRaTGNwSnVjNDNrR3BpTitUS3BlVXhrL2lTK0ta?=
 =?utf-8?B?aHFDYnlmVnlYS0xkdlU0L2lRdDZDeVZ0VHdiVllpbVJPaCsyejUvV3JoeE41?=
 =?utf-8?B?ZjVqT0dDMTRYYzdCNTdpc2Q3WlVNOXV1MmxUcWdQWlJsQTdWOFI4aGNRMkpl?=
 =?utf-8?B?dGRmSGZhOTN6OVlVSm9ZRzY3NGxtaVlHQ3RVeStOMkVuNUFvdG5zVFJoS0l1?=
 =?utf-8?B?WlFKejJ1RTNES0dSTWZFNTlTamdEMGZFeUJuay9KN2ZvWXVRcTlCVy9mQWp3?=
 =?utf-8?B?ZmhMQy9Ocmk1MzZYVHlxb1lhZWovQkdvM0FnVGszUDhiNXRCTVh6MHhPRGsz?=
 =?utf-8?B?cVRlalBudmRqUDZXaTdPaHFoNGREK0VjdmRqckNzV0swRTV5TStFRUdjbkNI?=
 =?utf-8?B?V2xHbW13WCs0aEhkcGcxM0M3aDJsU1lTc0I2S3ZJQnIzd01hUjI1SjdvTkhn?=
 =?utf-8?B?RUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d4c60835-7aaf-4352-4d5f-08db842f393c
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jul 2023 05:57:36.5707
 (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: 2ayKu8lAaizoxsFk1LWLad9zB8kFoXBwr4I7/AMkmdEUWMkcNUHNCFY50WEOnYqxywQ93OfPY06Wj0N+8wv/Fg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4810
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,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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


>> -               if (pageout_anon_only_filter && !folio_test_anon(folio))
>> +               /* Do not interfere with other mappings of this folio */
>> +               if (folio_mapcount(folio) != 1)
>>                         continue;
>>
>> -               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);
>> -               }
>> -
>> -               /*
>> -                * We are deactivating a folio for accelerating reclaiming.
>> -                * VM couldn't reclaim the folio unless we clear PG_young.
>> -                * As a side effect, it makes confuse idle-page tracking
>> -                * because they will miss recent referenced history.
>> -                */
>> -               folio_clear_referenced(folio);
>> -               folio_test_clear_young(folio);
>> -               if (folio_test_active(folio))
>> -                       folio_set_workingset(folio);
>> +pageout_cold_folio:
>>                 if (pageout) {
>>                         if (folio_isolate_lru(folio)) {
>>                                 if (folio_test_unevictable(folio))
>> @@ -529,8 +542,30 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>>                 arch_leave_lazy_mmu_mode();
>>                 pte_unmap_unlock(start_pte, ptl);
>>         }
>> -       if (pageout)
>> -               reclaim_pages(&folio_list);
>> +
>> +       if (pageout) {
>> +               LIST_HEAD(reclaim_list);
>> +
>> +               while (!list_empty(&folio_list)) {
>> +                       int refs;
>> +                       unsigned long flags;
>> +                       struct mem_cgroup *memcg = folio_memcg(folio);
>> +
>> +                       folio = lru_to_folio(&folio_list);
>> +                       list_del(&folio->lru);
>> +
>> +                       refs = folio_referenced(folio, 0, memcg, &flags);
>> +
>> +                       if ((flags & VM_LOCKED) || (refs == -1)) {
>> +                               folio_putback_lru(folio);
>> +                               continue;
>> +                       }
>> +
>> +                       folio_test_clear_referenced(folio);
>> +                       list_add(&folio->lru, &reclaim_list);
>> +               }
>> +               reclaim_pages(&reclaim_list);
>> +       }
> 
> i overlooked the chunk above -- it's unnecessary: after we split the
> large folio (and splice the base folios onto the same LRU list), we
> continue at the position of the first base folio because of:
> 
>   pte--;
>   addr -= PAGE_SIZE;
>   continue;
> 
> And then we do pte_mkold(), which takes care of the A-bit.
This patch moves the A-bit clear out of the folio isolation loop. So
even the folio is split and loop restarts from the first base folio,
the A-bit is not cleared. A-bit is only cleared in reclaim loop.

There is one option for A-bit clearing:
  - clear A-bit of base 4K page in isolation loop and leave large folio
    A-bit clearing to reclaim loop.

This patch didn't use it because don't want to introduce A-bit clearing
in two places. But I am open about clearing base 4K page A-bit cleared in
isolation loop. Thanks.


Regards
Yin, Fengwei