Linux-XFS Archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Wengang Wang <wen.gang.wang@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: make sure sb_fdblocks is non-negative
Date: Sat, 11 May 2024 11:17:33 +1000	[thread overview]
Message-ID: <Zj7HLZ5Mp5SjhvrH@dread.disaster.area> (raw)
In-Reply-To: <20240511003426.13858-1-wen.gang.wang@oracle.com>

On Fri, May 10, 2024 at 05:34:26PM -0700, Wengang Wang wrote:
> when writting super block to disk (in xfs_log_sb), sb_fdblocks is fetched from
> m_fdblocks without any lock. As m_fdblocks can experience a positive -> negativ
>  -> positive changing when the FS reaches fullness (see xfs_mod_fdblocks)
> So there is a chance that sb_fdblocks is negative, and because sb_fdblocks is
> type of unsigned long long, it reads super big. And sb_fdblocks being bigger
> than sb_dblocks is a problem during log recovery, xfs_validate_sb_write()
> complains.
> 
> Fix:
> As sb_fdblocks will be re-calculated during mount when lazysbcount is enabled,
> We just need to make xfs_validate_sb_write() happy -- make sure sb_fdblocks is
> not genative.

Ok, I have no problems with the change being made, but I'm unclear
on why we care if these values get logged as large positive numbers?

The comment above this code explains that these counts are known to
be inaccurate and so are not trusted. i.e. they will be corrected
post-log recovery if they are recovered from the log:

	 * Lazy sb counters don't update the in-core superblock so do that now.
         * If this is at unmount, the counters will be exactly correct, but at
         * any other time they will only be ballpark correct because of
         * reservations that have been taken out percpu counters. If we have an
         * unclean shutdown, this will be corrected by log recovery rebuilding
         * the counters from the AGF block counts.

IOWs journal recovery doesn't actually care what these values are,
so what actually goes wrong if this sum returns a negative value?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2024-05-11  1:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-11  0:34 [PATCH] xfs: make sure sb_fdblocks is non-negative Wengang Wang
2024-05-11  1:17 ` Dave Chinner [this message]
2024-05-13 17:06   ` Wengang Wang

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=Zj7HLZ5Mp5SjhvrH@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=wen.gang.wang@oracle.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).