Date:   Tue, 11 Jan 2022 21:02:45 -0800 (PST)
From:   Hugh Dickins <hughd@google.com>
To:     Yang Shi <shy828301@gmail.com>
cc:     Hugh Dickins <hughd@google.com>,
        Peng Liang <liangpeng10@huawei.com>,
        David Hildenbrand <david@redhat.com>,
        Linux MM <linux-mm@kvack.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        xiexiangyou@huawei.com, zhengchuan@huawei.com,
        wanghao232@huawei.com
Subject: Re: [RFC 0/1] memfd: Support mapping to zero page on reading
In-Reply-To: <CAHbLzkrt_3vA7h=jYf-RjPv3Myh-Duz62WqsjHmR30csM8MQzQ@mail.gmail.com>
Message-ID: <9e96fc57-6810-9efd-c1fc-4c8a54b125@google.com>
References: <20211222123400.1659635-1-liangpeng10@huawei.com> <4b1885b8-eb95-c50-2965-11e7c8efbf36@google.com> <CAHbLzkrt_3vA7h=jYf-RjPv3Myh-Duz62WqsjHmR30csM8MQzQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 11 Jan 2022, Yang Shi wrote:
> On Tue, Jan 11, 2022 at 6:30 PM Hugh Dickins <hughd@google.com> wrote:
> >
> > But I have to say that use of ZERO_PAGE for shmem/memfd/tmpfs read-fault
> > might (potentially) be very welcome.  Not as some MFD_ZEROPAGE special
> > case, but as how it would always work.  Deleting the shmem_recalc_inode()
> > cruft, which is there to correct accounting for the unmodified read-only
> > pages, after page reclaim has got around to freeing them later.
> 
> I'm wondering if we could use ZERO_PAGE for shmem_getpage() too so
> that we have simpler return value? Currently shmem_getpage() returns:
> #1. errno and NULL *pagep
> #2. 0 and valid *pagep
> #3. 0 and NULL *pagep if SGP_READ
> 
> Using ZERO_PAGE should be able to consolidate #2 and #3 so that we
> know there must be valid *pagep if 0 is returned.

At an earlier stage of mm/shmem.c's life, shmem_getpage() did return
ZERO_PAGE rather than NULL for that case; but I found it works out
better the way it is now (despite I'm not a fan of *pagep generally).

So I've no zest for messing with that now - though it's possible that
if we did extend the use of ZERO_PAGE, I'd look again and decide that
your change is then the best.

One reason for NULL rather than ZERO_PAGE was actually to help avoid
the cache-dirtying get_page/put_page on the thing; but that appears
to have gone missing at some point.  I have a patch in my tree which
fixes that, but held back from sending it in because it also used
iov_iter_zero() instead of copy_page_to_iter().  Obviously a good
improvement... except that whenever I timed it I found the opposite.
So, set aside on a shelf, to look into some other time.

> This should make read-fault use ZERO_PAGE automatically.

But I don't want to make read-fault use ZERO_PAGE automatically:
I want to be rather careful about that change!

Hugh