lkml.org 
[lkml]   [2022]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL] Folio fixes for 5.19
On Fri, Jun 10, 2022 at 12:56:48PM -0700, Linus Torvalds wrote:
> On Fri, Jun 10, 2022 at 12:22 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > - Don't release a folio while it's still locked
>
> Ugh.
>
> I *hate* this patch. It's just incredibly broken.
>
> Yes, I've pulled this, but I have looked at that readahead_folio()
> function before, and I have despised it before, but this patch really
> drove home how incredibly broken that function is.
>
> Honestly, readahead_folio() returns a folio *AFTER* it has dropped the
> ref to that folio.

OK, you caught me.

I realised (a little too late) that the rules around refcounts in
->readpage and ->readahead are different, and that creates pain for
people writing filesystems. For ->readahead, I stuck with the refcount
model that was in ->readpages (there is an extra refcount on the folio
and the filesystem must put it before it returns).

But I don't want to change the refcounting rules on a method without
changing something else about the method, because trying to find a
missing refcount change is misery. Anyway, my cunning thought was
that if I bundle the change to the refcount rule with the change
from readahead_page() to readahead_folio(), once all filesystems
are converted to readahead_folio(), I can pull the refcount game out
of readahead_folio() and do it in the caller where it belongs, all
transparent to the filesystems.

I think it's worth doing, because it's two fewer atomic ops per folio
that we read from a file. But I didn't think through the transition
process clearly enough, and right now it's a mess. How would you like
me to proceed?

(I don't think the erofs code has a bug because it doesn't remove
the folio from the pagecache while holding the lock -- the folio lock
prevents anyone _else_ from removing the folio from the pagecache,
so there must be a reference on the folio up until erofs calls
folio_unlock()).

\
 
 \ /
  Last update: 2022-06-10 23:40    [W:0.122 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site