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;
Date:   Thu, 10 Aug 2023 22:54:05 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Peter Xu <peterx@redhat.com>
Cc:     David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, linux-doc@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Hugh Dickins <hughd@google.com>,
        Ryan Roberts <ryan.roberts@arm.com>,
        Yin Fengwei <fengwei.yin@intel.com>,
        Yang Shi <shy828301@gmail.com>, Zi Yan <ziy@nvidia.com>
Subject: Re: [PATCH mm-unstable v1] mm: add a total mapcount for large folios
Message-ID: <ZNVcfdH8+N5Q83J/@casper.infradead.org>
References: <20230809083256.699513-1-david@redhat.com>
 <ZNQD4pxo8svpGmvX@x1n>
 <ZNRYx8GhYftE4Xeb@casper.infradead.org>
 <7e31254d-8889-7e79-50e1-2630bd493d59@redhat.com>
 <ZNVbIyHcqeKUDuSg@x1n>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZNVbIyHcqeKUDuSg@x1n>
Precedence: bulk

On Thu, Aug 10, 2023 at 05:48:19PM -0400, Peter Xu wrote:
> > Yes, that comment from Hugh primarily discusses how we could possibly
> > optimize the loop, and if relying on folio_nr_pages_mapped() to reduce the
> > iterations would be racy. As far as I can see, there are cases where "it
> > would be certainly a bad idea" :)
> 
> Is the race described about mapcount being changed right after it's read?
> Are you aware of anything specific that will be broken, and will be fixed
> with this patch?

The problem is that people check the mapcount while holding no locks;
not the PTL, not the page lock.  So it's an unfixable race.

> Having a total mapcount does sound helpful if partial folio is common
> indeed.
> 
> I'm curious whether that'll be so common after the large anon folio work -
> isn't it be sad if partial folio will be a norm?  It sounds to me that's
> the case when small page sizes should be used.. and it's prone to waste?

The problem is that entire_mapcount isn't really entire_mapcount.
It's pmd_mapcount.  I have had thoughts about using it as entire_mapcount,
but it gets gnarly when people do partial unmaps.  So the _usual_ case
ends up touching every struct page.  Which sucks.  Also it's one of the
things which stands in the way of shrinking struct page.

But it's kind of annoying to explain all of this to you individually.
There have been hundreds of emails about it over the last months on
this mailing list.  It would be nice if you could catch up instead of
jumping in.