From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: npiggin@kernel.de, xfs@oss.sgi.com
Subject: Re: [PATCH 2/2] xfs: ensure we mark all inodes in a freed cluster XFS_ISTALE
Date: Fri, 30 Jul 2010 06:27:46 -0400 [thread overview]
Message-ID: <20100730102746.GA10367@infradead.org> (raw)
In-Reply-To: <1280444146-14540-3-git-send-email-david@fromorbit.com>
On Fri, Jul 30, 2010 at 08:55:46AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Under heavy load parallel metadata loads (e.g. dbench), we can fail
> to mark all the inodes in a cluster being freed as XFS_ISTALE as we
> skip inodes we cannot get the XFS_ILOCK_EXCL or the flush lock on.
> When this happens and the inode cluster buffer has already been
> marked stale and freed, inode reclaim can try to write the inode out
> as it is dirty and not marked stale. This can result in writing th
> metadata to an freed extent, or in the case it has already
> been overwritten trigger a magic number check failure and return an
> EUCLEAN error such as:
>
> Filesystem "ram0": inode 0x442ba1 background reclaim flush failed with 117
>
> Fix this by ensuring that we hoover up all in memory inodes in the
> cluster and mark them XFS_ISTALE when freeing the cluster.
Why do you move the loop over the log items around? From all that
I can see the original place is much better as we just have to loop
over the items once. Then once we look up the inodes in memory
we skip over the inodes that already are stale, so the behaviour
should be the same. Also instead of the i-- and continue for the
lock failure an explicit goto retry would make it a lot more obvious.
The actual change of not skipping inodes looks good to me.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2010-06-09 2:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-29 22:55 [PATCH 0/2] xfs: fix bugs uncovered by dbench testing Dave Chinner
2010-07-29 22:55 ` [PATCH 1/2] xfs: unlock items before allowing the CIL to commit Dave Chinner
2010-07-30 8:49 ` Christoph Hellwig
2010-07-29 22:55 ` [PATCH 2/2] xfs: ensure we mark all inodes in a freed cluster XFS_ISTALE Dave Chinner
2010-07-30 10:27 ` Christoph Hellwig [this message]
2010-07-30 10:54 ` Dave Chinner
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=20100730102746.GA10367@infradead.org \
--to=hch@infradead.org \
--cc=david@fromorbit.com \
--cc=npiggin@kernel.de \
--cc=xfs@oss.sgi.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