From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58579C00528 for ; Wed, 2 Aug 2023 12:50:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234817AbjHBMuO (ORCPT ); Wed, 2 Aug 2023 08:50:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234901AbjHBMtt (ORCPT ); Wed, 2 Aug 2023 08:49:49 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id F0C6F2121; Wed, 2 Aug 2023 05:49:24 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2DD5A113E; Wed, 2 Aug 2023 05:50:07 -0700 (PDT) Received: from [10.57.77.90] (unknown [10.57.77.90]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AB4F03F5A1; Wed, 2 Aug 2023 05:49:21 -0700 (PDT) Message-ID: Date: Wed, 2 Aug 2023 13:49:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH 0/2] don't use mapcount() to check large folio sharing To: "Yin, Fengwei" , 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> From: Ryan Roberts In-Reply-To: <5f98748a-97ca-6426-1e24-a5675da75381@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > 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 >>>> >> >>