All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: xfs-oss <xfs@oss.sgi.com>
Subject: [PATCH, RFC] fix attr fit checking for filesystems which have lost their attr2
Date: Fri, 28 Mar 2008 23:56:25 -0500	[thread overview]
Message-ID: <47EDCBF9.4070102@sandeen.net> (raw)

Regarding the F8 corruption...  I have a pretty narrow testcase
now, and it turns out this was a bit of a perfect storm.

First, F8 shipped 2.6.23 which had the problem with sb_features2
padding out on 64-bit boxes, but this was ok because userspace
and kernelspace both did this, and it was properly mounting &
running as attr2.

However, hch came along in 2.6.24 and did some endian annotation 
for the superblock and in the process:
> A new helper xfs_sb_from_disk handles the other (read)
> direction and doesn't need the slightly hacky table-driven approach
> because we only ever read the full sb from disk.

However, this resulted in kernelspace behaving differently,
and now *missing* the attr2 flag in sb_features2, (actually
sb_bad_features2) so we mounted as if we had attr1.  Which 
is really supposed to be ok, IIRC, except in 
xfs_attr_shortform_bytesfit we return the default
fork offset value from the superblock, even if di_forkoff
is *already* set.  In the error case I had, di_forkoff was set
to 15 (from previously being attr2...) but we returned 14
(the mp default) and I think this is where things started
going wrong; I think this caused us to write an attr on top
of the extent data.

My understanding of this is that if di_forkoff is non-zero,
we should always be using it for space calculations, regardless
of whether we are mounted with attr2 or not...

and with that, how's this look, to be honest I haven't run it
through QA yet...

I'm not certain if xfs_bmap_compute_maxlevels() may lead
to similar problems....

------------------

always use di_forkoff for when checking for attr space

In the case where we mount a filesystem which was previously
using the attr2 format as attr1, returning the default
mp->m_attroffset instead of the per-inode di_forkoff for
inline attribute fit calculations may result in corruption,
if the existing "attr2" formatted attribute is already taking
more space than the default.

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---


Index: linux-2.6-git/fs/xfs/xfs_attr_leaf.c
===================================================================
--- linux-2.6-git.orig/fs/xfs/xfs_attr_leaf.c
+++ linux-2.6-git/fs/xfs/xfs_attr_leaf.c
@@ -166,7 +166,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t 
 
 	if (!(mp->m_flags & XFS_MOUNT_ATTR2)) {
 		if (bytes <= XFS_IFORK_ASIZE(dp))
-			return mp->m_attroffset >> 3;
+			return dp->i_d.di_forkoff;
 		return 0;
 	}

             reply	other threads:[~2008-03-29  4:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-29  4:56 Eric Sandeen [this message]
2008-03-31  0:52 ` [PATCH, RFC] fix attr fit checking for filesystems which have lost their attr2 Timothy Shimmin
2008-03-31  1:45   ` Eric Sandeen
2008-03-31  4:37   ` Timothy Shimmin
2008-03-31  6:11     ` Timothy Shimmin
2008-03-31  6:38     ` Timothy Shimmin
2008-03-31 13:32       ` Eric Sandeen

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=47EDCBF9.4070102@sandeen.net \
    --to=sandeen@sandeen.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.