Xfs fails in xfstests 013

Xfs fails in xfstests 013

[Lukáš Czerner]

Hi.

I've just run xfstest 013 on xfs and it fails with this backtrace


Jan 31 03:09:07 rhel6_vm1 kernel: ffff88020b6d1000: 78 78 78 78 78 78 78 78 78 78 2f 78 78 78 78 78  xxxxxxxxxx/xxxxx
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Internal error xfs_bmbt_verify at line 747 of file fs/xfs/xfs_bmap_btree.c.  Caller 0xffffffffa025486e
Jan 31 03:09:07 rhel6_vm1 kernel:
Jan 31 03:09:07 rhel6_vm1 kernel: Pid: 8909, comm: xfsaild/sda Tainted: GF            3.8.0-rc5-orig+ #8
Jan 31 03:09:07 rhel6_vm1 kernel: Call Trace:
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa022877f>] xfs_error_report+0x3f/0x50 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] ? xfs_bmbt_write_verify+0xe/0x10 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02287ee>] xfs_corruption_error+0x5e/0x90 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0254710>] xfs_bmbt_verify+0x80/0x1d0 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] ? xfs_bmbt_write_verify+0xe/0x10 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] xfs_bmbt_write_verify+0xe/0x10 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02269a5>] _xfs_buf_ioapply+0x65/0x150 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8108d170>] ? try_to_wake_up+0x2b0/0x2b0
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02270a5>] ? xfs_bdstrat_cb+0x65/0xd0 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0226fef>] xfs_buf_iorequest+0x4f/0xa0 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02270a5>] xfs_bdstrat_cb+0x65/0xd0 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0227218>] __xfs_buf_delwri_submit+0x108/0x200 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02273c0>] ? xfs_buf_delwri_submit_nowait+0x20/0x30 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02273c0>] xfs_buf_delwri_submit_nowait+0x20/0x30 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0282376>] xfsaild_push+0x1b6/0x520 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0282778>] xfsaild+0x98/0x130 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02826e0>] ? xfsaild_push+0x520/0x520 [xfs]
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8107968e>] kthread+0xce/0xe0
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff810795c0>] ? kthread_freezable_should_stop+0x70/0x70
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8155cdec>] ret_from_fork+0x7c/0xb0
Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff810795c0>] ? kthread_freezable_should_stop+0x70/0x70
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Corruption detected. Unmount and run xfs_repair
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): xfs_do_force_shutdown(0x8) called from line 1358 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffffa0226a85
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Corruption of in-memory data detected.  Shutting down filesystem
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Please umount the filesystem and rectify the problem(s)
Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): xfs_log_force: error 5 returned.


I can reproduce it every time with xfstest 013 on 3.8.0-rc5.

-Lukas

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Ben Myers]

Hey Lukas,

On Thu, Jan 31, 2013 at 09:19:23AM +0100, Lukáš Czerner wrote:
> I've just run xfstest 013 on xfs and it fails with this backtrace
> 
> 
> Jan 31 03:09:07 rhel6_vm1 kernel: ffff88020b6d1000: 78 78 78 78 78 78 78 78 78 78 2f 78 78 78 78 78  xxxxxxxxxx/xxxxx
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Internal error xfs_bmbt_verify at line 747 of file fs/xfs/xfs_bmap_btree.c.  Caller 0xffffffffa025486e
> Jan 31 03:09:07 rhel6_vm1 kernel:
> Jan 31 03:09:07 rhel6_vm1 kernel: Pid: 8909, comm: xfsaild/sda Tainted: GF            3.8.0-rc5-orig+ #8
> Jan 31 03:09:07 rhel6_vm1 kernel: Call Trace:
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa022877f>] xfs_error_report+0x3f/0x50 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] ? xfs_bmbt_write_verify+0xe/0x10 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02287ee>] xfs_corruption_error+0x5e/0x90 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0254710>] xfs_bmbt_verify+0x80/0x1d0 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] ? xfs_bmbt_write_verify+0xe/0x10 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa025486e>] xfs_bmbt_write_verify+0xe/0x10 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02269a5>] _xfs_buf_ioapply+0x65/0x150 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8108d170>] ? try_to_wake_up+0x2b0/0x2b0
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02270a5>] ? xfs_bdstrat_cb+0x65/0xd0 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0226fef>] xfs_buf_iorequest+0x4f/0xa0 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02270a5>] xfs_bdstrat_cb+0x65/0xd0 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0227218>] __xfs_buf_delwri_submit+0x108/0x200 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02273c0>] ? xfs_buf_delwri_submit_nowait+0x20/0x30 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02273c0>] xfs_buf_delwri_submit_nowait+0x20/0x30 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0282376>] xfsaild_push+0x1b6/0x520 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa0282778>] xfsaild+0x98/0x130 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffffa02826e0>] ? xfsaild_push+0x520/0x520 [xfs]
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8107968e>] kthread+0xce/0xe0
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff810795c0>] ? kthread_freezable_should_stop+0x70/0x70
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff8155cdec>] ret_from_fork+0x7c/0xb0
> Jan 31 03:09:07 rhel6_vm1 kernel: [<ffffffff810795c0>] ? kthread_freezable_should_stop+0x70/0x70
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Corruption detected. Unmount and run xfs_repair
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): xfs_do_force_shutdown(0x8) called from line 1358 of file fs/xfs/xfs_buf.c.  Return address = 0xffffffffa0226a85
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Corruption of in-memory data detected.  Shutting down filesystem
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Please umount the filesystem and rectify the problem(s)
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): xfs_log_force: error 5 returned.
> 
> 
> I can reproduce it every time with xfstest 013 on 3.8.0-rc5.

I haven't been able to reproduce this with test 013.  Could you grab a
metadump?

Thanks,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Dave Chinner]

On Thu, Jan 31, 2013 at 09:19:23AM +0100, Lukáš Czerner wrote:
> Hi.
> 
> I've just run xfstest 013 on xfs and it fails with this backtrace
> 
> 
> Jan 31 03:09:07 rhel6_vm1 kernel: ffff88020b6d1000: 78 78 78 78 78 78 78 78 78 78 2f 78 78 78 78 78  xxxxxxxxxx/xxxxx
> Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Internal error xfs_bmbt_verify at line 747 of file fs/xfs/xfs_bmap_btree.c.  Caller 0xffffffffa025486e

Same problem Cai Qian just reported - a remote symlink with a bmbt
verifier attached to it.

> I can reproduce it every time with xfstest 013 on 3.8.0-rc5.

And the configuration you are testing?

I'd suggest that you add an:

	ASSERT(bp->b_ops == NULL);

into xfs_symlink() after the xfs_trans_get_buf() call in the remote
symlink crate loop, because the problem occurring implies that the
code is getting a new buffer with a stale ops structure on it
(though I can't see how that is possible right now).

You should probably put the same assert into
xfs_inactive_symlink_rmt() between the get buf and the
xfs_trans_binval() call, and into xfs_readlink_bmap() after the
contents of a symlink are read from disk.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1832 bytes --]

On Fri, 1 Feb 2013, Dave Chinner wrote:

> Date: Fri, 1 Feb 2013 11:29:27 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: xfs@oss.sgi.com
> Subject: Re: Xfs fails in xfstests 013
> 
> On Thu, Jan 31, 2013 at 09:19:23AM +0100, Lukáš Czerner wrote:
> > Hi.
> > 
> > I've just run xfstest 013 on xfs and it fails with this backtrace
> > 
> > 
> > Jan 31 03:09:07 rhel6_vm1 kernel: ffff88020b6d1000: 78 78 78 78 78 78 78 78 78 78 2f 78 78 78 78 78  xxxxxxxxxx/xxxxx
> > Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Internal error xfs_bmbt_verify at line 747 of file fs/xfs/xfs_bmap_btree.c.  Caller 0xffffffffa025486e
> 
> Same problem Cai Qian just reported - a remote symlink with a bmbt
> verifier attached to it.
> 
> > I can reproduce it every time with xfstest 013 on 3.8.0-rc5.
> 
> And the configuration you are testing?

The system is x86_64 KVM guest and I am using the following config

export TEST_DEV=/dev/sda
export TEST_DIR=/mnt/test
export SCRATCH_DEV=/dev/sdb
export SCRATCH_MNT=/mnt/test1
export MKFS_OPTIONS="-f "
export FSTYP=xfs

Where /dev/sda and /dev/sdb are 50GB in size and those are exported
from the host linear logical volumes.

> 
> I'd suggest that you add an:
> 
> 	ASSERT(bp->b_ops == NULL);
> 
> into xfs_symlink() after the xfs_trans_get_buf() call in the remote
> symlink crate loop, because the problem occurring implies that the
> code is getting a new buffer with a stale ops structure on it
> (though I can't see how that is possible right now).
> 
> You should probably put the same assert into
> xfs_inactive_symlink_rmt() between the get buf and the
> xfs_trans_binval() call, and into xfs_readlink_bmap() after the
> contents of a symlink are read from disk.

Ok, I'll try that and let you know.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3481 bytes --]

On Fri, 1 Feb 2013, Lukáš Czerner wrote:

> Date: Fri, 1 Feb 2013 11:58:04 +0100 (CET)
> From: Lukáš Czerner <lczerner@redhat.com>
> To: Dave Chinner <david@fromorbit.com>
> Cc: Lukáš Czerner <lczerner@redhat.com>, xfs@oss.sgi.com
> Subject: Re: Xfs fails in xfstests 013
> 
> On Fri, 1 Feb 2013, Dave Chinner wrote:
> 
> > Date: Fri, 1 Feb 2013 11:29:27 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukáš Czerner <lczerner@redhat.com>
> > Cc: xfs@oss.sgi.com
> > Subject: Re: Xfs fails in xfstests 013
> > 
> > On Thu, Jan 31, 2013 at 09:19:23AM +0100, Lukáš Czerner wrote:
> > > Hi.
> > > 
> > > I've just run xfstest 013 on xfs and it fails with this backtrace
> > > 
> > > 
> > > Jan 31 03:09:07 rhel6_vm1 kernel: ffff88020b6d1000: 78 78 78 78 78 78 78 78 78 78 2f 78 78 78 78 78  xxxxxxxxxx/xxxxx
> > > Jan 31 03:09:07 rhel6_vm1 kernel: XFS (sda): Internal error xfs_bmbt_verify at line 747 of file fs/xfs/xfs_bmap_btree.c.  Caller 0xffffffffa025486e
> > 
> > Same problem Cai Qian just reported - a remote symlink with a bmbt
> > verifier attached to it.
> > 
> > > I can reproduce it every time with xfstest 013 on 3.8.0-rc5.
> > 
> > And the configuration you are testing?
> 
> The system is x86_64 KVM guest and I am using the following config
> 
> export TEST_DEV=/dev/sda
> export TEST_DIR=/mnt/test
> export SCRATCH_DEV=/dev/sdb
> export SCRATCH_MNT=/mnt/test1
> export MKFS_OPTIONS="-f "
> export FSTYP=xfs
> 
> Where /dev/sda and /dev/sdb are 50GB in size and those are exported
> from the host linear logical volumes.
> 
> > 
> > I'd suggest that you add an:
> > 
> > 	ASSERT(bp->b_ops == NULL);
> > 
> > into xfs_symlink() after the xfs_trans_get_buf() call in the remote
> > symlink crate loop, because the problem occurring implies that the
> > code is getting a new buffer with a stale ops structure on it
> > (though I can't see how that is possible right now).
> > 
> > You should probably put the same assert into
> > xfs_inactive_symlink_rmt() between the get buf and the
> > xfs_trans_binval() call, and into xfs_readlink_bmap() after the
> > contents of a symlink are read from disk.
> 
> Ok, I'll try that and let you know.

This is what I've done:

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index d95f565..22590ef 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -89,6 +89,7 @@ xfs_readlink_bmap(
                        xfs_buf_relse(bp);
                        goto out;
                }
+               ASSERT(bp->b_ops == NULL);
                if (pathlen < byte_cnt)
                        byte_cnt = pathlen;
                pathlen -= byte_cnt;
@@ -313,6 +314,7 @@ xfs_inactive_symlink_rmt(
                        error = ENOMEM;
                        goto error1;
                }
+               ASSERT(bp->b_ops == NULL);
                xfs_trans_binval(tp, bp);
        }
        /*
@@ -1535,6 +1537,7 @@ xfs_symlink(
                                error = ENOMEM;
                                goto error2;
                        }
+                       ASSERT(bp->b_ops == NULL);
                        if (pathlen < byte_cnt) {
                                byte_cnt = pathlen;
                        }

and I have not hit the assert. However I think that I know what's
going on. The verifier is obviously not attached when the symlink is
created, however it is attached when security attributes are created
for the symlink by xfs_init_security().

Thanks!
-Lukas

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Dave Chinner]

On Fri, Feb 01, 2013 at 03:33:06PM +0100, Lukáš Czerner wrote:
> On Fri, 1 Feb 2013, Lukáš Czerner wrote:
> and I have not hit the assert. However I think that I know what's
> going on. The verifier is obviously not attached when the symlink is
> created, however it is attached when security attributes are created
> for the symlink by xfs_init_security().

Which means the bug is in xfs_bmap_add_attrfork_local() in calling
xfs_bmap_local_to_extents(). i.e it assumes that the local data is
either directory data or extent data, and is attaching extent tree
verifiers to the buffer that contains a remote symlink. Thinking
time now needed, as this affects the symlink CRC changes as well.

Thanks for the debug help, Lukas.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Dave Chinner]

On Mon, Feb 04, 2013 at 10:00:13AM +1100, Dave Chinner wrote:
> On Fri, Feb 01, 2013 at 03:33:06PM +0100, Lukáš Czerner wrote:
> > On Fri, 1 Feb 2013, Lukáš Czerner wrote:
> > and I have not hit the assert. However I think that I know what's
> > going on. The verifier is obviously not attached when the symlink is
> > created, however it is attached when security attributes are created
> > for the symlink by xfs_init_security().
> 
> Which means the bug is in xfs_bmap_add_attrfork_local() in calling
> xfs_bmap_local_to_extents(). i.e it assumes that the local data is
> either directory data or extent data, and is attaching extent tree
> verifiers to the buffer that contains a remote symlink. Thinking
> time now needed, as this affects the symlink CRC changes as well.
> 
> Thanks for the debug help, Lukas.

Lukas, can you check whether the patch below fixes the problem you
saw?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com


xfs: xfs_bmap_add_attrfork_local is too generic

From: Dave Chinner <dchinner@redhat.com>

When we are converting local data to an extent format as a result of
adding an attribute, the type of data contained in the local fork
determines the behaviour that needs to occur.

xfs_bmap_add_attrfork_local() already handles the directory data
case specially by using S_ISDIR() and calling out to
xfs_dir2_sf_to_block(), but with verifiers we now need to handle
each different type of metadata specially and different metadata
formats require different verifiers (and eventually block header
initialisation).

There is only a single place that we add and attribute fork to
the inode, but that is in the attribute code and it knows nothing
about the specific contents of the data fork. It is only the case of
local data that is the issue here, so adding code to hadnle this
case in the attribute specific code is wrong. Hence we are really
stuck trying to detect the data fork contents in
xfs_bmap_add_attrfork_local() and performing the correct callout
there.

Luckily the current cases can be determined by S_IS* macros, and we
can push the work off to data specific callouts, but each of those
callouts does a lot of work in common with
xfs_bmap_local_to_extents(). The only reason that this fails for
symlinks right now is is that xfs_bmap_local_to_extents() assumes
the data fork contains extent data, and so attaches a a bmap extent
data verifier to the buffer and simply copies the data fork
information straight into it.

To fix this, allow us to pass a "formatting" callback into
xfs_bmap_local_to_extents() which is responsible for setting the
buffer type, initialising it and copying the data fork contents over
to the new buffer. This allows callers to specify how they want to
format the new buffer (which is necessary for the upcoming CRC
enabled metadata blocks) and hence make xfs_bmap_local_to_extents()
useful for any type of data fork content.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_bmap.c |  114 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 93 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 491f35e..b44af92 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -147,7 +147,10 @@ xfs_bmap_local_to_extents(
 	xfs_fsblock_t	*firstblock,	/* first block allocated in xaction */
 	xfs_extlen_t	total,		/* total blocks needed by transaction */
 	int		*logflagsp,	/* inode logging flags */
-	int		whichfork);	/* data or attr fork */
+	int		whichfork,	/* data or attr fork */
+	void		(*init_fn)(struct xfs_buf *bp,
+				   struct xfs_inode *ip,
+				   struct xfs_ifork *ifp));
 
 /*
  * Search the extents list for the inode, for the extent containing bno.
@@ -357,7 +360,42 @@ xfs_bmap_add_attrfork_extents(
 }
 
 /*
- * Called from xfs_bmap_add_attrfork to handle local format files.
+ * Block initialisation functions for local to extent format conversion.
+ * As these get more complex, they will be moved to the relevant files,
+ * but for now they are too simple to worry about.
+ */
+STATIC void
+xfs_bmap_local_to_extents_init_fn(
+	struct xfs_buf		*bp,
+	struct xfs_inode	*ip,
+	struct xfs_ifork	*ifp)
+{
+	bp->b_ops = &xfs_bmbt_buf_ops;
+	memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
+}
+
+STATIC void
+xfs_symlink_local_to_remote(
+	struct xfs_buf		*bp,
+	struct xfs_inode	*ip,
+	struct xfs_ifork	*ifp)
+{
+	/* remote symlink blocks are not verifiable until CRCs come along */
+	bp->b_ops = NULL;
+	memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
+}
+
+/*
+ * Called from xfs_bmap_add_attrfork to handle local format files. Each
+ * different data fork content type needs a different callout to do the
+ * conversion. Some are basic and only require special block initialisation
+ * callouts for the data formating, others (directories) are so specialised they
+ * handle everything themselves.
+ *
+ * XXX (dgc): investigate whether directory conversion can use the generic
+ * formatting callout. It should be possible - it's just a very complex
+ * formatter. it would also require passing the transaction through to the init
+ * function.
  */
 STATIC int					/* error */
 xfs_bmap_add_attrfork_local(
@@ -368,25 +406,29 @@ xfs_bmap_add_attrfork_local(
 	int			*flags)		/* inode logging flags */
 {
 	xfs_da_args_t		dargs;		/* args for dir/attr code */
-	int			error;		/* error return value */
-	xfs_mount_t		*mp;		/* mount structure pointer */
 
 	if (ip->i_df.if_bytes <= XFS_IFORK_DSIZE(ip))
 		return 0;
+
 	if (S_ISDIR(ip->i_d.di_mode)) {
-		mp = ip->i_mount;
 		memset(&dargs, 0, sizeof(dargs));
 		dargs.dp = ip;
 		dargs.firstblock = firstblock;
 		dargs.flist = flist;
-		dargs.total = mp->m_dirblkfsbs;
+		dargs.total = ip->i_mount->m_dirblkfsbs;
 		dargs.whichfork = XFS_DATA_FORK;
 		dargs.trans = tp;
-		error = xfs_dir2_sf_to_block(&dargs);
-	} else
-		error = xfs_bmap_local_to_extents(tp, ip, firstblock, 1, flags,
-			XFS_DATA_FORK);
-	return error;
+		return xfs_dir2_sf_to_block(&dargs);
+	}
+
+	if (S_ISLNK(ip->i_d.di_mode))
+		return xfs_bmap_local_to_extents(tp, ip, firstblock, 1,
+						 flags, XFS_DATA_FORK,
+						 xfs_symlink_local_to_remote);
+
+	return xfs_bmap_local_to_extents(tp, ip, firstblock, 1, flags,
+					 XFS_DATA_FORK,
+					 xfs_bmap_local_to_extents_init_fn);
 }
 
 /*
@@ -3219,7 +3261,10 @@ xfs_bmap_local_to_extents(
 	xfs_fsblock_t	*firstblock,	/* first block allocated in xaction */
 	xfs_extlen_t	total,		/* total blocks needed by transaction */
 	int		*logflagsp,	/* inode logging flags */
-	int		whichfork)	/* data or attr fork */
+	int		whichfork,
+	void		(*init_fn)(struct xfs_buf *bp,
+				   struct xfs_inode *ip,
+				   struct xfs_ifork *ifp))
 {
 	int		error;		/* error return value */
 	int		flags;		/* logging flags returned */
@@ -3239,12 +3284,12 @@ xfs_bmap_local_to_extents(
 		xfs_buf_t	*bp;	/* buffer for extent block */
 		xfs_bmbt_rec_host_t *ep;/* extent record pointer */
 
+		ASSERT((ifp->if_flags &
+			(XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) == XFS_IFINLINE);
 		memset(&args, 0, sizeof(args));
 		args.tp = tp;
 		args.mp = ip->i_mount;
 		args.firstblock = *firstblock;
-		ASSERT((ifp->if_flags &
-			(XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) == XFS_IFINLINE);
 		/*
 		 * Allocate a block.  We know we need only one, since the
 		 * file currently fits in an inode.
@@ -3258,17 +3303,20 @@ xfs_bmap_local_to_extents(
 		}
 		args.total = total;
 		args.minlen = args.maxlen = args.prod = 1;
-		if ((error = xfs_alloc_vextent(&args)))
+		error = xfs_alloc_vextent(&args);
+		if (error)
 			goto done;
-		/*
-		 * Can't fail, the space was reserved.
-		 */
+
+		/* Can't fail, the space was reserved. */
 		ASSERT(args.fsbno != NULLFSBLOCK);
 		ASSERT(args.len == 1);
 		*firstblock = args.fsbno;
 		bp = xfs_btree_get_bufl(args.mp, tp, args.fsbno, 0);
-		bp->b_ops = &xfs_bmbt_buf_ops;
-		memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
+
+		/* initialise the block and copy the data */
+		init_fn(bp, ip, ifp);
+
+		/* account for the change in fork size and log everything */
 		xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1);
 		xfs_bmap_forkoff_reset(args.mp, ip, whichfork);
 		xfs_idata_realloc(ip, -ifp->if_bytes, whichfork);
@@ -4915,8 +4963,32 @@ xfs_bmapi_write(
 	XFS_STATS_INC(xs_blk_mapw);
 
 	if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
+		/*
+		 * XXX (dgc): This assumes we are only called for inodes that
+		 * contain content neutral data in local format. Anything that
+		 * contains caller-specific data in local format that needs
+		 * transformation to move to a block format needs to do the
+		 * conversion to extent format itself.
+		 *
+		 * Directory data forks and attribute forks handle this
+		 * themselves, but with the addition of metadata verifiers every
+		 * data fork in local format now contains caller specific data
+		 * and as such conversion through this function is likely to be
+		 * broken.
+		 *
+		 * The only likely user of this branch is for remote symlinks,
+		 * but we cannot overwrite the data fork contents of the symlink
+		 * (EEXIST occurs higher up the stack) and so it will never go
+		 * from local format to extent format here. Hence I don't think
+		 * this branch is ever executed intentionally and we should
+		 * consider removing it and asserting that xfs_bmapi_write()
+		 * cannot be called directly on local format forks. i.e. callers
+		 * are completely responsible for local to extent format
+		 * conversion, not xfs_bmapi_write().
+		 */
 		error = xfs_bmap_local_to_extents(tp, ip, firstblock, total,
-						  &bma.logflags, whichfork);
+					&bma.logflags, whichfork,
+					xfs_bmap_local_to_extents_init_fn);
 		if (error)
 			goto error0;
 	}

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: Xfs fails in xfstests 013

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1323 bytes --]

On Mon, 11 Feb 2013, Dave Chinner wrote:

> Date: Mon, 11 Feb 2013 09:16:00 +1100
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: xfs@oss.sgi.com
> Subject: Re: Xfs fails in xfstests 013
> 
> On Mon, Feb 04, 2013 at 10:00:13AM +1100, Dave Chinner wrote:
> > On Fri, Feb 01, 2013 at 03:33:06PM +0100, Lukáš Czerner wrote:
> > > On Fri, 1 Feb 2013, Lukáš Czerner wrote:
> > > and I have not hit the assert. However I think that I know what's
> > > going on. The verifier is obviously not attached when the symlink is
> > > created, however it is attached when security attributes are created
> > > for the symlink by xfs_init_security().
> > 
> > Which means the bug is in xfs_bmap_add_attrfork_local() in calling
> > xfs_bmap_local_to_extents(). i.e it assumes that the local data is
> > either directory data or extent data, and is attaching extent tree
> > verifiers to the buffer that contains a remote symlink. Thinking
> > time now needed, as this affects the symlink CRC changes as well.
> > 
> > Thanks for the debug help, Lukas.
> 
> Lukas, can you check whether the patch below fixes the problem you
> saw?

Hi Dave,

from my testing it seems that ti fixes the problem. Can not
reproduce it anymore with the patch applied.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs