From: "Barry Naujok" <bnaujok@melbourne.sgi.com>
To: xfs@oss.sgi.com, 'xfs-dev' <xfs-dev@sgi.com>
Subject: [PATCH] xfs_repair - move realtime extent processing to a separate function
Date: Fri, 13 Apr 2007 14:22:10 +1000 [thread overview]
Message-ID: <200704130415.OAA16550@larry.melbourne.sgi.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 577 bytes --]
While changing the process_bmbt_reclist_int() function, I observed a
realtime check inside the block map get/set state loop which is quite
CPU intensive. Upon further investigation, this loop is not used at
all for realtime extents and that the two types of extents are pretty
much processed exclusively.
So, I simplified the functionality by moving the realtime extent
processing into it's own function and fixing a bug at the same time
when it comes to realtime inodes with attributes (it was comparing
attr extents to the realtime volume bmap instead of the normal bmap).
[-- Attachment #2: separate_rt_extent_processing.patch --]
[-- Type: application/octet-stream, Size: 8428 bytes --]
Index: repair/xfsprogs/repair/dinode.c
===================================================================
--- repair.orig/xfsprogs/repair/dinode.c 2007-04-13 13:07:16.000000000 +1000
+++ repair/xfsprogs/repair/dinode.c 2007-04-13 14:15:33.920960345 +1000
@@ -537,6 +537,121 @@
return (val); \
} while (0)
+static int
+process_rt_rec(
+ xfs_mount_t *mp,
+ xfs_bmbt_rec_32_t *rp,
+ xfs_ino_t ino,
+ xfs_drfsbno_t *tot,
+ int check_dups)
+{
+ xfs_dfsbno_t b;
+ xfs_drtbno_t ext;
+ xfs_dfilblks_t c; /* count */
+ xfs_dfsbno_t s; /* start */
+ xfs_dfiloff_t o; /* offset */
+ int state;
+ int flag; /* extent flag */
+ int pwe; /* partially-written extent */
+
+ convert_extent(rp, &o, &s, &c, &flag);
+
+ /*
+ * check numeric validity of the extent
+ */
+ if (s >= mp->m_sb.sb_rblocks) {
+ do_warn(_("inode %llu - bad rt extent start block number "
+ "%llu, offset %llu\n"), ino, s, o);
+ return 1;
+ }
+ if (s + c - 1 >= mp->m_sb.sb_rblocks) {
+ do_warn(_("inode %llu - bad rt extent last block number %llu, "
+ "offset %llu\n"), ino, s + c - 1, o);
+ return 1;
+ }
+ if (s + c - 1 < s) {
+ do_warn(_("inode %llu - bad rt extent overflows - start %llu, "
+ "end %llu, offset %llu\n"),
+ ino, s, s + c - 1, o);
+ return 1;
+ }
+
+ /*
+ * verify that the blocks listed in the record
+ * are multiples of an extent
+ */
+ if (XFS_SB_VERSION_HASEXTFLGBIT(&mp->m_sb) == 0 &&
+ (s % mp->m_sb.sb_rextsize != 0 ||
+ c % mp->m_sb.sb_rextsize != 0)) {
+ do_warn(_("malformed rt inode extent [%llu %llu] (fs rtext "
+ "size = %u)\n"), s, c, mp->m_sb.sb_rextsize);
+ return 1;
+ }
+
+ /*
+ * set the appropriate number of extents
+ */
+ for (b = s; b < s + c; b += mp->m_sb.sb_rextsize) {
+ ext = (xfs_drtbno_t) b / mp->m_sb.sb_rextsize;
+ pwe = XFS_SB_VERSION_HASEXTFLGBIT(&mp->m_sb) && flag &&
+ (b % mp->m_sb.sb_rextsize != 0);
+
+ if (check_dups == 1) {
+ if (search_rt_dup_extent(mp, ext) && !pwe) {
+ do_warn(_("data fork in rt ino %llu claims "
+ "dup rt extent, off - %llu, "
+ "start - %llu, count %llu\n"),
+ ino, o, s, c);
+ return 1;
+ }
+ continue;
+ }
+
+ state = get_rtbno_state(mp, ext);
+
+ switch (state) {
+ case XR_E_FREE:
+ case XR_E_UNKNOWN:
+ set_rtbno_state(mp, ext, XR_E_INUSE);
+ break;
+
+ case XR_E_BAD_STATE:
+ do_error(_("bad state in rt block map %llu\n"),
+ ext);
+
+ case XR_E_FS_MAP:
+ case XR_E_INO:
+ case XR_E_INUSE_FS:
+ do_error(_("data fork in rt inode %llu found "
+ "metadata block %llu in rt bmap\n"),
+ ino, ext);
+
+ case XR_E_INUSE:
+ if (pwe)
+ break;
+
+ case XR_E_MULT:
+ set_rtbno_state(mp, ext, XR_E_MULT);
+ do_warn(_("data fork in rt inode %llu claims "
+ "used rt block %llu\n"),
+ ino, ext);
+ return 1;
+
+ case XR_E_FREE1:
+ default:
+ do_error(_("illegal state %d in rt block map "
+ "%llu\n"), state, b);
+ }
+ }
+
+ /*
+ * bump up the block counter
+ */
+ *tot += c;
+
+ return 0;
+}
+
/*
* return 1 if inode should be cleared, 0 otherwise
* if check_dups should be set to 1, that implies that
@@ -560,7 +675,6 @@
int whichfork)
{
xfs_dfsbno_t b;
- xfs_drtbno_t ext;
xfs_dfilblks_t c; /* count */
xfs_dfilblks_t cp = 0; /* prev count */
xfs_dfsbno_t s; /* start */
@@ -572,7 +686,6 @@
int i;
int state;
int flag; /* extent flag */
- int pwe; /* partially-written extent */
xfs_dfsbno_t e;
xfs_agnumber_t agno;
xfs_agblock_t agbno;
@@ -615,28 +728,22 @@
o, s, ino);
PROCESS_BMBT_UNLOCK_RETURN(1);
}
- if (type == XR_INO_RTDATA) {
- if (s >= mp->m_sb.sb_rblocks) {
- do_warn(
- _("inode %llu - bad rt extent start block number %llu, offset %llu\n"),
- ino, s, o);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- if (s + c - 1 >= mp->m_sb.sb_rblocks) {
- do_warn(
- _("inode %llu - bad rt extent last block number %llu, offset %llu\n"),
- ino, s + c - 1, o);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- if (s + c - 1 < s) {
- do_warn(
- _("inode %llu - bad rt extent overflows - start %llu, end %llu, "
- "offset %llu\n"),
- ino, s, s + c - 1, o);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- } else {
- switch (verify_dfsbno_range(mp, s, c)) {
+
+ if (type == XR_INO_RTDATA && whichfork == XFS_DATA_FORK) {
+ /*
+ * realtime bitmaps don't use AG locks, so returning
+ * immediately is fine for this code path.
+ */
+ if (process_rt_rec(mp, rp, ino, tot, check_dups))
+ return 1;
+ /*
+ * skip rest of loop processing since that's
+ * all for regular file forks and attr forks
+ */
+ continue;
+ }
+
+ switch (verify_dfsbno_range(mp, s, c)) {
case XR_DFSBNORANGE_VALID:
break;
case XR_DFSBNORANGE_BADSTART:
@@ -656,109 +763,13 @@
"offset %llu\n"),
ino, s, s + c - 1, o);
PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- if (o >= fs_max_file_offset) {
- do_warn(
+ }
+ if (o >= fs_max_file_offset) {
+ do_warn(
_("inode %llu - extent offset too large - start %llu, count %llu, "
"offset %llu\n"),
- ino, s, c, o);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- }
-
- /*
- * realtime file data fork
- */
- if (type == XR_INO_RTDATA && whichfork == XFS_DATA_FORK) {
- /*
- * XXX - verify that the blocks listed in the record
- * are multiples of an extent
- */
- if (XFS_SB_VERSION_HASEXTFLGBIT(&mp->m_sb) == 0
- && (s % mp->m_sb.sb_rextsize != 0 ||
- c % mp->m_sb.sb_rextsize != 0)) {
- do_warn(
- _("malformed rt inode extent [%llu %llu] (fs rtext size = %u)\n"),
- s, c, mp->m_sb.sb_rextsize);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
-
- /*
- * XXX - set the appropriate number of extents
- */
- for (b = s; b < s + c; b += mp->m_sb.sb_rextsize) {
- ext = (xfs_drtbno_t) b / mp->m_sb.sb_rextsize;
- if (XFS_SB_VERSION_HASEXTFLGBIT(&mp->m_sb) &&
- flag && (b % mp->m_sb.sb_rextsize != 0)) {
- pwe = 1;
- } else {
- pwe = 0;
- }
-
- if (check_dups == 1) {
- if (search_rt_dup_extent(mp, ext) &&
- !pwe) {
- do_warn(
- _("data fork in rt ino %llu claims dup rt extent, off - %llu, "
- "start - %llu, count %llu\n"),
- ino, o, s, c);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
- continue;
- }
-
- state = get_rtbno_state(mp, ext);
-
- switch (state) {
- case XR_E_FREE:
-/* XXX - turn this back on after we
- run process_rtbitmap() in phase2
- do_warn(
- _("%s fork in rt ino %llu claims free rt block %llu\n"),
- forkname, ino, ext);
-*/
- /* fall through ... */
- case XR_E_UNKNOWN:
- set_rtbno_state(mp, ext, XR_E_INUSE);
- break;
- case XR_E_BAD_STATE:
- do_error(
- _("bad state in rt block map %llu\n"), ext);
- abort();
- break;
- case XR_E_FS_MAP:
- case XR_E_INO:
- case XR_E_INUSE_FS:
- do_error(
- _("%s fork in rt inode %llu found metadata block %llu in %s bmap\n"),
- forkname, ino, ext, ftype);
- case XR_E_INUSE:
- if (pwe)
- break;
- case XR_E_MULT:
- set_rtbno_state(mp, ext, XR_E_MULT);
- do_warn(
- _("%s fork in rt inode %llu claims used rt block %llu\n"),
- forkname, ino, ext);
- PROCESS_BMBT_UNLOCK_RETURN(1);
- case XR_E_FREE1:
- default:
- do_error(
- _("illegal state %d in %s block map %llu\n"),
- state, ftype, b);
- }
- }
-
- /*
- * bump up the block counter
- */
- *tot += c;
-
- /*
- * skip rest of loop processing since that's
- * all for regular file forks and attr forks
- */
- continue;
+ ino, s, c, o);
+ PROCESS_BMBT_UNLOCK_RETURN(1);
}
@@ -793,15 +804,6 @@
continue;
}
- /* FIX FOR BUG 653709 -- EKN
- * realtime attribute fork, should be valid block number
- * in regular data space, not realtime partion.
- */
- if (type == XR_INO_RTDATA && whichfork == XFS_ATTR_FORK) {
- if (mp->m_sb.sb_agcount < agno)
- PROCESS_BMBT_UNLOCK_RETURN(1);
- }
-
/* Process in chunks of 16 (XR_BB_UNIT/XR_BB)
* for common XR_E_UNKNOWN to XR_E_INUSE transition
*/
next reply other threads:[~2007-04-13 4:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-04-13 4:22 Barry Naujok [this message]
2007-04-13 7:26 ` [PATCH] xfs_repair - move realtime extent processing to a separate function Christoph Hellwig
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=200704130415.OAA16550@larry.melbourne.sgi.com \
--to=bnaujok@melbourne.sgi.com \
--cc=xfs-dev@sgi.com \
--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