Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <fd389af5-d949-43dc-9a35-d53112fe4a60@intel.com>
Date:   Thu, 19 Oct 2023 20:07:02 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm: migrate: record the mlocked page status to remove
 unnecessary lru drain
To:     Baolin Wang <baolin.wang@linux.alibaba.com>,
        "Huang, Ying" <ying.huang@intel.com>, Zi Yan <ziy@nvidia.com>
CC:     <akpm@linux-foundation.org>, <mgorman@techsingularity.net>,
        <hughd@google.com>, <vbabka@suse.cz>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>
References: <64899ad0bb78cde88b52abed1a5a5abbc9919998.1697632761.git.baolin.wang@linux.alibaba.com>
 <1F80D8DA-8BB5-4C7E-BC2F-030BF52931F7@nvidia.com>
 <87il73uos1.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <2ad721be-b81e-d279-0055-f995a8cfe180@linux.alibaba.com>
 <27f40fc2-806a-52a9-3697-4ed9cd7081d4@intel.com>
 <e8099116-6f78-cb4a-5036-1d7e38b63e52@linux.alibaba.com>
Content-Language: en-US
From:   "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <e8099116-6f78-cb4a-5036-1d7e38b63e52@linux.alibaba.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?cndza0VHK0VqK1ZJVVFlbldBeWFleFJ0Y3pGUTljSE5vVDE1bloyU1EwekQ4?=
 =?utf-8?B?ZXlhb1lHK2d4VkN5YjZ3OGh0K3R0UG1MVWJFN2VPdGx5MmYvY29KQXdUcnJZ?=
 =?utf-8?B?Ry9NYjN5bllpSzhhdm4zMGVJeHlMNm9oZHJtWjZtU2pTU0dVU01MR1pscVZn?=
 =?utf-8?B?emZjUWhSV0pmaEFoRGNzSE1iVVdRN24zZG0yczhVODRkSTNyWmJuaUV6ZTg2?=
 =?utf-8?B?TU11QWlBamRYeHVZcXl6RnJMRFlXY1pKNzV6QU1SRUVNd21JNUFjWHR0K3M3?=
 =?utf-8?B?VlZRTXR5U04ySkRvUThhK1ExM3hwZ3hxSnBuMzdWQ0c1dTZkd3BHU3dsdDZh?=
 =?utf-8?B?KzhMU1dnVEhSQVdGcTZnbUlMSndjeWgzVEh5VkFmWXJhd3U2VzVYN3RZZlVG?=
 =?utf-8?B?QVRydkFlMEVnRExuMTNscnJyNzJYRTQzNWF1WTd0c3o2V0FkUXFkLzdaYmxh?=
 =?utf-8?B?ZEgzTHpwZHRIUDFPdi9NVXhJQ1F3OGJDVDRmZE1FUFNOaTMzdHhNSFNnR0lG?=
 =?utf-8?B?dkh2VEdTU1ZCZ25tVW5rdTZzRkxwL2ttbHptdExxOU1qdk1VTWpuclRyNzNL?=
 =?utf-8?B?ZW8rZ1RWa2JUV1BUb3NQa1Z0eUFqR2FDeXhqd0duWDZlR05raUpGTW1KbHE3?=
 =?utf-8?B?QUFDS0NOMWFkWUsxL1pOME1uVjhxNlJVT2p4L3V5QVlJN09mVm1MSTMwTTdv?=
 =?utf-8?B?SzVqMTNoWldpeGxtU0t5UUtEcExUV3Q1Q2c0V05PU2F0T0IrakxNS0JmVE9h?=
 =?utf-8?B?eENkUlh5UnlzcTA0dW5KSGh5azIrbzY2bURJNDIzU3BRcFA3N2YwQXA3aFRH?=
 =?utf-8?B?QXA5QjJJR1NTTnUzU25mNFcxOUtyUFJHSmFGOUFwTWQ3S05iTnZCaFRTelds?=
 =?utf-8?B?OG9haDdZc0h2bW1CWUwxZ1pwM1p1aUtpK2RYakdmcUo4SXRua1hSeXZBY2F3?=
 =?utf-8?B?YnZEelh0Y2FWZ0YrQlRIL1Bnb3pxMXowRVN1SkpxbUt0dmNRTmxDWE5vZmVl?=
 =?utf-8?B?Vy9VS1R2V3B4VmhkWHdWNGhIWkFZNThWamRaNGEwZndEdFJnbzh3SjJQdXRq?=
 =?utf-8?B?TWxwK040dW5hTDFCRmx0MU95Rjh2aEpRRCtScTI4THlKUCswZ0JNMHhjaFhK?=
 =?utf-8?B?NGZIaXFaWWJ2eDR3RDFTeURlbGZoMWsyTEZuM2h6QTlZNGZQcVdlZE5pKzFs?=
 =?utf-8?B?WWZkM3Ezb1RFSUptUUs3ZVZiS01TUmdNa1FhT241QVJ5ZGlnMUwvaUFXU2Vs?=
 =?utf-8?B?K3gzcllYVGttVXZ0YUxqK1QvWEY0U2ZUZElNUGEyaTBXd1lycERvQ0d0U0NE?=
 =?utf-8?B?R0NyZ256VDhDZGFRRkxJZkh0dTNYTjRlUkdwUzdhNkc2S0pzaEpicHFJTnRt?=
 =?utf-8?B?ZEE5dFlpZ2JCOFllM2pDVUJpU2xWMHlsaDZTVWMzYU1iUXpVRGY3TExZaU1B?=
 =?utf-8?B?MEM0b055QVpjM2FVbWhUNlJFSFozYWh0a09iVHRsVjJyb0g5eEp3S0Mwb05n?=
 =?utf-8?B?TkhvekNsZUwyVUlCOWd4LzQ0RlB0ZkluVmtHMmcxRXZ4REh5SWFHcWdvWmUr?=
 =?utf-8?B?eDBFOUF0MFgyTUdxRE5Sb3dXZDY4Y0hXZndxTHNwcGNtVFhFZ3FnOXNOL0cv?=
 =?utf-8?B?bGFqelJTV1ZzaG1lK2pYQnN2dkM0c1ZVU2dUMW1vUXBWMHljamppOEwwMVpN?=
 =?utf-8?B?MVdLSGtPbzBlUGhqdTdIR3E5MnNyc0wxclBYVzdrdU5QeUtGcXNlWkRVSUpI?=
 =?utf-8?B?TXVrbXREbW03M3BSZXV4VkhDVFZzZVMyMURjVnd6NFh4OFMyQzg5RCtSNlVl?=
 =?utf-8?B?Rkl5MWRWRjlJWEZuTXk1NDgyUEdDbVBFWk1tQURiVHVUQ2RBNlNSTFAva3lS?=
 =?utf-8?B?bGRiZ1dIYkZHYlhIaER0N0plV3Y1N1o0U3JmNXJzb2VRWjRBSVpjVm4wZDZY?=
 =?utf-8?B?aXJkdUF1MHJ2Rlc4c1FOeVNOdHRuNnllTmhkUHM0aDBjMTJxeGFEZnFwWjQw?=
 =?utf-8?B?Z2pySGE3OXNWZVRleHIxVTQ2bXNFTmt3UGZlNGdHM2pWM0ZpMmRhRm1ESnlF?=
 =?utf-8?B?b0RWMUFJc0xpT1FVbTF5ZjQwR0FGbjJoSExsbWVqTkc4Y0wyMnQ4ZkMxS3pD?=
 =?utf-8?Q?TSjr9A3mxX2SgaJ4NDTNk9VHX?=
X-MS-Exchange-CrossTenant-Network-Message-Id: de4b183d-fa04-40e6-4fae-08dbd09beb4b
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2023 12:07:09.0395
 (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: je2nd7PAIsSwLhq9sqDWeNNIcmBcVMJxKy5MxGc2yEnZJWUK2vBSLVwizLRRgJGeUdS9o3fGXFuaPqaT36/unQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7867
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,
        SPF_NONE,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
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 19 Oct 2023 05:09:34 -0700 (PDT)


On 10/19/2023 4:51 PM, Baolin Wang wrote:
> 
> 
> On 10/19/2023 4:22 PM, Yin Fengwei wrote:
>> Hi Baolin,
>>
>> On 10/19/23 15:25, Baolin Wang wrote:
>>>
>>>
>>> On 10/19/2023 2:09 PM, Huang, Ying wrote:
>>>> Zi Yan <ziy@nvidia.com> writes:
>>>>
>>>>> On 18 Oct 2023, at 9:04, Baolin Wang wrote:
>>>>>
>>>>>> When doing compaction, I found the lru_add_drain() is an obvious hotspot
>>>>>> when migrating pages. The distribution of this hotspot is as follows:
>>>>>>      - 18.75% compact_zone
>>>>>>         - 17.39% migrate_pages
>>>>>>            - 13.79% migrate_pages_batch
>>>>>>               - 11.66% migrate_folio_move
>>>>>>                  - 7.02% lru_add_drain
>>>>>>                     + 7.02% lru_add_drain_cpu
>>>>>>                  + 3.00% move_to_new_folio
>>>>>>                    1.23% rmap_walk
>>>>>>               + 1.92% migrate_folio_unmap
>>>>>>            + 3.20% migrate_pages_sync
>>>>>>         + 0.90% isolate_migratepages
>>>>>>
>>>>>> The lru_add_drain() was added by commit c3096e6782b7 ("mm/migrate:
>>>>>> __unmap_and_move() push good newpage to LRU") to drain the newpage to LRU
>>>>>> immediately, to help to build up the correct newpage->mlock_count in
>>>>>> remove_migration_ptes() for mlocked pages. However, if there are no mlocked
>>>>>> pages are migrating, then we can avoid this lru drain operation, especailly
>>>>>> for the heavy concurrent scenarios.
>>>>>
>>>>> lru_add_drain() is also used to drain pages out of folio_batch. Pages in folio_batch
>>>>> have an additional pin to prevent migration. See folio_get(folio); in folio_add_lru().
>>>>
>>>> lru_add_drain() is called after the page reference count checking in
>>>> move_to_new_folio().  So, I don't this is an issue.
>>>
>>> Agree. The purpose of adding lru_add_drain() is to address the 'mlock_count' issue for mlocked pages. Please see commit c3096e6782b7 and related comments. Moreover I haven't seen an increase in the number of page migration failures due to page reference count checking after this patch.
>>
>> I agree with your. My understanding also is that the lru_add_drain() is only needed
>> for mlocked folio to correct mlock_count. Like to hear the confirmation from Huge.
>>
>>
>> But I have question: why do we need use page_was_mlocked instead of check
>> folio_test_mlocked(src)? Does page migration clear the mlock flag? Thanks.
> 
> Yes, please see the call trace: try_to_migrate_one() ---> page_remove_rmap() ---> munlock_vma_folio().

Yes. This will clear mlock bit.

What about set dst folio mlocked if source is before try_to_migrate_one()? And
then check whether dst folio is mlocked after? And need clear mlocked if migration
fails. I suppose the change is minor. Just a thought. Thanks.


Regards
Yin, Fengwei