linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: Freeing page flags
Date: Fri, 13 May 2022 04:39:28 +0100	[thread overview]
Message-ID: <Yn3S8A9I/G5F4u80@casper.infradead.org> (raw)
In-Reply-To: <Yn3FZSZbEDssbRnk@localhost.localdomain>

On Thu, May 12, 2022 at 10:41:41PM -0400, Josef Bacik wrote:
> On Thu, May 12, 2022 at 09:54:59PM +0100, Matthew Wilcox wrote:
> > The LWN writeup [1] on merging the MGLRU reminded me that I need to send
> > out a plan for removing page flags that we can do without.
> > 
> > 1. PG_error.  It's basically useless.  If the page was read successfully,
> > PG_uptodate is set.  If not, PG_uptodate is clear.  The page cache
> > doesn't use PG_error.  Some filesystems do, and we need to transition
> > them away from using it.
> >
> 
> What about writes?  A cursory look shows we don't clear Uptodate if we fail to
> write, which is correct I think.  The only way to indicate we had a write error
> to check later is the page error.

On encountering a write error, we're supposed to call mapping_set_error(),
not SetPageError().

> > 2. PG_private.  This tells us whether we have anything stored at
> > page->private.  We can just check if page->private is NULL or not.
> > No need to have this extra bit.  Again, there may be some filesystems
> > that are a bit wonky here, but I'm sure they're fixable.
> > 
> 
> At least for Btrfs we serialize the page->private with the private_lock, so we
> could probably just drop PG_private, but it's kind of nice to check first before
> we have to take the spin lock.  I suppose we can just do
> 
> if (page->private)
> 	// do lock and check thingy

That's my hope!  I think btrfs is already using folio_attach_private() /
attach_page_private(), which makes everything easier.  Some filesystems
still manipulate page->private and PagePrivate by hand.


  reply	other threads:[~2022-05-13  3:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-12 20:54 Matthew Wilcox
2022-05-13  2:41 ` Josef Bacik
2022-05-13  3:39   ` Matthew Wilcox [this message]
2022-05-13  9:40     ` Luís Henriques
2022-05-13 12:53       ` Matthew Wilcox
2022-05-13 13:18         ` Luís Henriques
2022-05-13 13:21         ` Jeff Layton
2022-05-13 13:38           ` Matthew Wilcox
2022-05-13 13:57             ` Jeff Layton
2022-05-17  0:34         ` Xiubo Li
2022-05-13 13:17     ` Josef Bacik
2022-05-13  3:46 ` Yu Zhao
2022-05-14  6:10 ` Eric Biggers
2022-05-23  6:38 ` Mike Rapoport
2022-06-10 17:39   ` David Hildenbrand
2022-05-30 16:54 ` David Hildenbrand

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=Yn3S8A9I/G5F4u80@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.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