Re: [PATCH v2 1/1] Documentation: update pagemap with shmem exceptions

From: Tiberiu Georgescu <tiberiu.georgescu@nutanix.com>
To: Peter Xu <peterx@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jonathan Corbet <corbet@lwn.net>,
	"david@redhat.com" <david@redhat.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ivan Teterevkov <ivan.teterevkov@nutanix.com>,
	Florian Schmidt <flosch@nutanix.com>,
	"Carl Waldspurger [C]" <carl.waldspurger@nutanix.com>,
	Jonathan Davies <jond@nutanix.com>
Subject: Re: [PATCH v2 1/1] Documentation: update pagemap with shmem exceptions
Date: Tue, 21 Sep 2021 17:09:48 +0000	[thread overview]
Message-ID: <B110C852-5688-4A40-A7C8-099638D10AB7@nutanix.com> (raw)
In-Reply-To: <YUoIk247W1Sbt6Lf@t490s>

> On 21 Sep 2021, at 17:30, Peter Xu <peterx@redhat.com> wrote:
> 
> On Tue, Sep 21, 2021 at 04:08:29PM +0000, Tiberiu Georgescu wrote:
>> Hmmm, so if we put emphasis on the accuracy of swap info, or accuracy in
>> general, we need to use frontswap. Otherwise, mincore() could suffer from
>> race conditions, and mark pages in the swap cache as being present.
> 
> IMHO it's not a race condition, but by design.
> 
> Quotting again from the mincore() man page:
> 
>       The vec argument must point to an array containing at least
>       (length+PAGE_SIZE-1) / PAGE_SIZE bytes.  On return, the least
>       significant bit of each byte will be set if the corresponding page is
>       currently resident in memory, and be clear otherwise.
> 
> I think "within swap cache" does mean that it still resides in memory, so it's
> not violating what it's designed to me, at least from the manpage.

I agree mincore() is doing what it is designed to do. I did not express my
thoughts correctly.
> 
>> 
>> Do you reckon this info (frontswap for mincore) should be present in
>> the pagemap docs? I wouldn't want to bloat the section either.
> 
> I don't think the type of swap matters in this document, but imho mentioning
> mincore() as the alternative to fetch swap is still meaningful as that's what's
> missing for pagemap right now on shmem typed memories.
> 
> Even if it cannot identify some cases between "page presents", "page stays in
> page cache", or "page stays in swap cache", it'll still be good enough to me.

Yeah, it should be good enough mostly. But I feel that providing an exact
algorithm on how to separate the cases is not appropriate anymore,
as there are still exceptions, and the "perfect" algorithm needs frontswap
as a backend, which can be a fairly prohibitive precondition.

Maybe I should just note all these tools down instead and provide a brief
explanation of their usage and their downsides in the context of pagemap
information.

Thanks a lot for all suggestions! I'll cook up a v3 shortly.

Kind regards,
Tibi