Re: PagePrivate handling

[Gao Xiang <hsiangkao@aol.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8\"", Size: 2355 bytes --]

On Wed, Oct 14, 2020 at 04:38:36PM +0100, Matthew Wilcox wrote:
> On Wed, Oct 14, 2020 at 10:50:51AM -0400, Chris Mason wrote:
> > On 14 Oct 2020, at 9:49, Matthew Wilcox wrote:

...

> > > 
> > > Also ... do we really need to increment the page refcount if we have
> > > PagePrivate set?  I'm not awfully familiar with the buffercache -- is
> > > it possible we end up in a situation where a buffer, perhaps under I/O,
> > > has the last reference to a struct page?  It seems like that reference
> > > is
> > > always put from drop_buffers() which is called from
> > > try_to_free_buffers()
> > > which is always called by someone who has a reference to a struct page
> > > that they got from the pagecache.  So what is this reference count for?
> > 
> > I’m not sure what we gain by avoiding the refcount bump?  Many filesystems
> > use the pattern of: “put something in page->private, free that thing in
> > releasepage.† Without the refcount bump it feels like we’d have more magic
> > to avoid freeing the page without leaking things in page->private.  I think
> > the extra ref lets the FS crowd keep our noses out of the MM more often, so
> > it seems like a net positive to me.
> 
> The question is whether the "thing" in page->private can ever have the
> last reference on a struct page.  Gao says erofs can be in that situation,
> so never mind this change.

Add some words, just my thought... we have a management structure which could
store PagePrivate page cache pages, !PagePrivate page cache pages, and non-page
cache pages which are directly from buddy system.

and I knew the extra refcount rule for PagePrivate from the beginning (since
the rule is quite stable for many many years (I remembered from 200x introduced
by akpm?) so I designed the whole workflow to handle these different types of
pages in the management structure based on this rule and to make reclaim &
migrate work for all page cache pages properly.). I think many modules think
the rule is stable as well ... anyway, I think there's always be another way
to handle the same thing if the refcount rule is changed, yet I need to
revisit all current logic and do proper changes. And I think many modules
(including out-of-tree modules) could be impacted as well... anyway...

Thanks,
Gao Xiang

>