On Mon, Apr 08, 2024 at 10:07:01PM +0200, David Hildenbrand wrote:
> On 03.04.24 19:18, Matthew Wilcox (Oracle) wrote:
> > Replace the use of pages with folios. Saves a few calls to
> > compound_head() and removes some uses of obsolete functions.
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > ---
>
> [...]
>
> > - if (page_count(page) !=
> > - 1 + page_mapcount(page) + page_has_private(page)) {
> > + if (folio_ref_count(folio) !=
> > + 1 + folio_mapcount(folio) + folio_test_private(folio)) {
> > result = SCAN_PAGE_COUNT;
>
> Just stumbled over that: page_has_private() would have checked PG_private
> and PG_private_2. The corresponding replacement would have been
> folio_has_private(). folio_test_private() only checks PG_private.
>
> Are we sure that we no longer have to check PG_private_2 here? pagecache ...
> so I have no clue :)
Oh man. Vishal just asked me about this in our meeting and I struggled
to explain the whole horrid history. The short answer is that this
almost certainly fixes a bug.
We have a rule (which most filesystem developers don't know about) that
attaching private data to the folio should increase its refcount by one.
This is handled by folio_attach_private(). But there's no corresponding
rule that setting PG_private_2 should also increment the refcount.
So checking PG_private_2 is wrong here because a folio with PG_private_2
set and PG_private clear will not have its refcount increased.
There are longer versions of this answer, and if you keep asking, I'll
give them ;-P
