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: <837ba176-c97f-f81b-c044-eb6aa3d88bb7@intel.com>
Date:   Wed, 2 Aug 2023 20:55:49 +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: [PATCH 0/2] don't use mapcount() to check large folio sharing
Content-Language: en-US
To:     Ryan Roberts <ryan.roberts@arm.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>,
        <akpm@linux-foundation.org>, <willy@infradead.org>,
        <vishal.moola@gmail.com>, <wangkefeng.wang@huawei.com>,
        <minchan@kernel.org>, <yuzhao@google.com>, <david@redhat.com>,
        <shy828301@gmail.com>
References: <20230728161356.1784568-1-fengwei.yin@intel.com>
 <3bbfde16-ced1-dca8-6a3f-da893e045bc5@arm.com>
 <56c8f4f9-b54b-b0bb-250c-ec8643accfc7@intel.com>
 <3541d2de-5cf8-2f84-8153-277e2bfc0101@arm.com>
 <5f98748a-97ca-6426-1e24-a5675da75381@intel.com>
 <a590da86-0c42-7d46-d320-c661a59a46c1@arm.com>
From:   "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <a590da86-0c42-7d46-d320-c661a59a46c1@arm.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?V1BDc3F5SVRWUUJDQXFNR0FzZ2hGNHljcm55RmMzT0dqM2JkelBBbGNoODQz?=
 =?utf-8?B?Rzg4eXdnU2lVRjZNNEhmUUg0UHp2c2tRM3N3enJxS2t2d3p5TUs4R1FOeFFX?=
 =?utf-8?B?YU5SRlVqUCtxTGk4aVdEYnQrOFcyTk5FaVRVMXFmRWlTUFRQSy9QbkgwcEY3?=
 =?utf-8?B?TVNnSERXeEFPRHNlZXorY3lvanJUdXN1RFNFVng4R01EQmNMZ051UCsrMWRN?=
 =?utf-8?B?NVJjNUtOQkI2QUdMZ1BVamhYTldZSXR3TDR6M1pPcVN5bE05aXdlelNJZStN?=
 =?utf-8?B?NFVIaEFFdzAyQmVrSFNTZ0JWVXNyT1Y1Mm1LWTN1eVcvN1NaWmVMRjJvN1BG?=
 =?utf-8?B?QktXdUI4SFp0M0IxSW9HK1YreHlMbmN2L01FQ2tyWFNKOXU2eWMrRHNQakdO?=
 =?utf-8?B?M1ZobmZyVjFvQWV3Q25zZWxWKzdLWWpkOStES0NLUmYra3JVWjhQWTQ0elVp?=
 =?utf-8?B?UWFCMG1CWnA1Nk03M0tkLzIxaFBJcUtFM1J1bUhENHU3aG51QUNkQnI0ZVFX?=
 =?utf-8?B?R3E5SytPcDhnNGx3UnJveDFhbDA2eG1UV0trdFhjN082UC8ya0ZWNXcwdEhC?=
 =?utf-8?B?VVBwSW9lSC9sMFc4QjJPdUFJRHVYODVBek9BbGIxRDhoK0NCUmdIK0JtM016?=
 =?utf-8?B?dmNRMXc4RGFiOHBndXF1dlhPdk92dXFYYUdyeEkzY25QMjRNcG1waC9OazRp?=
 =?utf-8?B?TkFvN3V5VHJWejloZ0FuQTUzSlBFMHpaZVQ4bVBtNEUzbG91aGJERUkrdFo5?=
 =?utf-8?B?U2x3UGN3NklsNDNidTdGUXdvQUtCSDkyRDE1U2FUbURobjJaTFB6UlB1TjU1?=
 =?utf-8?B?YnlIRXZycUdNNXdCbGVpSVJPZ3M2NFNUakJORXpaaUpTbU50ekdQdGRXSGF1?=
 =?utf-8?B?WnN1ZmxjNUdQMkt3a1F5aFV1VVFJUW96a1dBUjgrbjgvSjRNNWpXL0lxREJL?=
 =?utf-8?B?NzEydXY5SjQ4WVdmeTZ5MGNmQlcxTEd3TjNHZmc1Q3M2b3Rmd1RudnFaUXl5?=
 =?utf-8?B?ektGcSswaGExNkRqNHZKZitXOFp5eWRyak9BRUZEV2ppSndkNnNTdzZOOGxk?=
 =?utf-8?B?RGNydlRRSkpJNnVBSFVqZ3VFRzhGaFdlU0JSdnpvZGZkMjVsQkhiVW9lZWVi?=
 =?utf-8?B?SjF5WDl0SHRzYnFFWS91bEFkRFRKTTV2Mml3NHNQcXBheGhVOFkyOFBQUVl3?=
 =?utf-8?B?TGJBamVXYmpFOTdLUXdUcEZ4QnFuMmNUcVJnN1JjRzBxMFhxckNocXpJT0pI?=
 =?utf-8?B?TGJDTmZleGpERmpWNjRrQVZ6NnpibUtvQW1UZFZmVm1IQ05EQjAxV3o3aG1Q?=
 =?utf-8?B?NmZOdEZxczZpdm54SDdzaHZ1WExZbElOWlBXdTVOS2VuZWZUNWFQcXZuK0tC?=
 =?utf-8?B?NVgwWVYzeDdRRnFDV2l4NURINDRZS0kwRHE1STZJdE9vbVM5WlFVVlNOTnFL?=
 =?utf-8?B?U3JOTzJDNEd6SmI1QURzSy9xUjdQTjVhR0RQUlJQcGJ4a2I5S0xNeStBem1V?=
 =?utf-8?B?ejZ0NjJKLzM4aXpsOUgvT1pxMHVTTmNkbFJpdkMyQXRZNDcvenIrYndyMmR0?=
 =?utf-8?B?MThOcWNHNjd6dS9mc3BpSjNTb2J0UUlybktuekx2SHJHRGlaWjhFbDFqOGpH?=
 =?utf-8?B?RUFRQ1gyc08zdy9GaVJSZGRvMDlNRjdudiswUkFmQ295a0tLVk42ZitYWHFJ?=
 =?utf-8?B?MHdlK09VbzhiaEJkWnB5NEFPQzNWMUNpSFlCQVdnRUNoMDl6TEZZbzFMbTJY?=
 =?utf-8?B?OXJteVhyVldMMCtsazdkR2NLQU1pTDA4VldNTjlZbEFlVXB1aFVtZXlsS0xv?=
 =?utf-8?B?SE9uaVBFeGhmd1oyV09zMXMwV2MxOTd2bTJDZU43T0NDclFuZEE3TG5IZ1RL?=
 =?utf-8?B?OEF3SlZIRHBCMFNnbVl1ZFpIb2s1aDc3bExxMmxQQ0pLYTdvdE5PcEFCT3VK?=
 =?utf-8?B?NmpiRHV0bmRlMlJ2bDh2bWI1ZzJpSWhXRWxSeVFNbkozMUlGT0lra01DMEta?=
 =?utf-8?B?TzF1WlQvZ2tycWszRUZHNVA0cVMrYkFiWlc0Z2JpaW55SnQyMjBnZ2g4RUwv?=
 =?utf-8?B?QWpYSENoTUMrYURydXdwVzhMRXdaR2E0UGFhbitHcUZtTEtOeHhRaXJibTV6?=
 =?utf-8?Q?eI96GO/J2AzxiyVBlTOPXqj4X?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 283d2600-a2f4-4792-9e0e-08db9357d3ee
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Aug 2023 12:56:03.1047
 (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: v7D8ndrjIgleFy1ZslKAo9NK056AEPO1IJZ800OJcEfJi53FO1EPLbFApzgtm9yMeh1pZ5urH3CgRKRKBu7VTw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4735
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 8/2/2023 8:49 PM, Ryan Roberts wrote:
> On 02/08/2023 13:42, Yin, Fengwei wrote:
>>
>>
>> On 8/2/2023 8:40 PM, Ryan Roberts wrote:
>>> On 02/08/2023 13:35, Yin, Fengwei wrote:
>>>>
>>>>
>>>> On 8/2/2023 6:27 PM, Ryan Roberts wrote:
>>>>> On 28/07/2023 17:13, Yin Fengwei wrote:
>>>>>> In madvise_cold_or_pageout_pte_range() and madvise_free_pte_range(),
>>>>>> folio_mapcount() is used to check whether the folio is shared. But it's
>>>>>> not correct as folio_mapcount() returns total mapcount of large folio.
>>>>>>
>>>>>> Use folio_estimated_sharers() here as the estimated number is enough.
>>>>>>
>>>>>> Yin Fengwei (2):
>>>>>>   madvise: don't use mapcount() against large folio for sharing check
>>>>>>   madvise: don't use mapcount() against large folio for sharing check
>>>>>>
>>>>>>  mm/huge_memory.c | 2 +-
>>>>>>  mm/madvise.c     | 6 +++---
>>>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>>>>
>>>>>
>>>>> As a set of fixes, I agree this is definitely an improvement, so:
>>>>>
>>>>> Reviewed-By: Ryan Roberts
>>>> Thanks.
>>>>
>>>>>
>>>>>
>>>>> But I have a couple of comments around further improvements;
>>>>>
>>>>> Once we have the scheme that David is working on to be able to provide precise
>>>>> exclusive vs shared info, we will probably want to move to that. Although that
>>>>> scheme will need access to the mm_struct of a process known to be mapping the
>>>>> folio. We have that info, but its not passed to folio_estimated_sharers() so we
>>>>> can't just reimplement folio_estimated_sharers() - we will need to rework these
>>>>> call sites again.
>>>> Yes. This could be extra work. Maybe should delay till David's work is done.
>>>
>>> What you have is definitely an improvement over what was there before. And is
>>> probably the best we can do without David's scheme. So I wouldn't delay this.
>>> Just pointing out that we will be able to make it even better later on (if
>>> David's stuff goes in).
>> Yes. I agree that we should wait for David's work ready and do fix based on that.
> 
> I was suggesting the opposite - not waiting. Then we can do separate improvement
> later.
Let's wait for David's work ready. Otherwise, some other similar fix could be
submitted. Unfortunately, we can't build folio_estimated_sharers() based on
folio_maybe_mapped_shared() because latter one requires the extra parameter.


Regards
Yin, Fengwei

> 
>>
>>
>> Regards
>> Yin, Fengwei
>>
>>>
>>>>
>>>>>
>>>>> Given the aspiration for most of the memory to be large folios going forwards,
>>>>> wouldn't it be better to avoid splitting the large folio where the large folio
>>>>> is mapped entirely within the range of the madvise operation? Sorry if this has
>>>>> already been discussed and decided against - I didn't follow the RFC too
>>>>> closely. Or perhaps you plan to do this as a follow up?
>>>> Yes. We are on same page. RFC patchset did that. But there are some other opens
>>>> on the RFC. So I tried to submit this part of change which is bug fix. The other
>>>> thing left in RFC is optimization (avoid split large folio if we can).
>>>>
>>>>
>>>> Regards
>>>> Yin, Fengwei
>>>>
>>>>>
>>>>> Thanks,
>>>>> Ryan
>>>>>
>>>
>>>
>