linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] Folio fixes for 5.19
Date: Fri, 10 Jun 2022 16:27:24 -0700	[thread overview]
Message-ID: <CAHk-=wgzenicjKjXJnbmh7Nf-Y2aX=Kc46OsskSrKcpuozjFsg@mail.gmail.com> (raw)
In-Reply-To: <YqO6FaO0/I9Ateze@casper.infradead.org>

On Fri, Jun 10, 2022 at 2:40 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> 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.

Hmm. Any reason why that can't be done right now? Aren't we basically
converted already?

Yeah, yeah, there's a couple of users of readahead_page() left, but if
cleaning up the folio case requires some fixup to those, then that
sounds better than the current "folio interface is very messy".

> (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()).

Ahh. Ugh. And I guess the whole "clearing the lock bit is the last
time we touch the page flags" and "folio_wake_bit() is very careful to
only touch the external waitqueue" so that there can be no nasty races
with somebody coming in *exactly* as the folio is unlocked.

This has been subtle before, but I think we did allow it exactly for
this kind of reason. I've swapped out the details.

            Linus


  reply	other threads:[~2022-06-10 23:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-10 19:22 Matthew Wilcox
2022-06-10 19:56 ` Linus Torvalds
2022-06-10 21:39   ` Matthew Wilcox
2022-06-10 23:27     ` Linus Torvalds [this message]
2022-06-10 19:58 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgzenicjKjXJnbmh7Nf-Y2aX=Kc46OsskSrKcpuozjFsg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox