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 >
