[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9e96fc57-6810-9efd-c1fc-4c8a54b125@google.com>
Date: Tue, 11 Jan 2022 21:02:45 -0800 (PST)
From: Hugh Dickins <hughd@...gle.com>
To: Yang Shi <shy828301@...il.com>
cc: Hugh Dickins <hughd@...gle.com>,
Peng Liang <liangpeng10@...wei.com>,
David Hildenbrand <david@...hat.com>,
Linux MM <linux-mm@...ck.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
xiexiangyou@...wei.com, zhengchuan@...wei.com,
wanghao232@...wei.com
Subject: Re: [RFC 0/1] memfd: Support mapping to zero page on reading
On Tue, 11 Jan 2022, Yang Shi wrote:
> On Tue, Jan 11, 2022 at 6:30 PM Hugh Dickins <hughd@...gle.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
Powered by blists - more mailing lists