Skip to contentSkip to conversation

btrfs: fix wrong ctime when adding link

PATCH In review Miao Xie 4 participants 5 messages May 20, 2010 – May 24, 2010 Btrfs file system

lore
Share & tools
lore

Miao Xie

author
#0 lore raw

PATCH 7/10 btrfs: fix wrong ctime when adding link

the ctime of file has not been updated when I create a link for it.

Steps to reproduce:

 # touch file1
 # stat -c %Z file1
 1273592239
 # link flink1 file1
 # stat -c %Z file1
 1273592239             <-- have not been updated

This patch fix this problem.

---
 fs/btrfs/inode.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
fs/btrfs/inode.c +6 -2
index a85b90c..5271887 100644
@@ -4218,8 +4218,12 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 
 		btrfs_i_size_write(parent_inode, parent_inode->i_size +
 				   name_len * 2);
-		parent_inode->i_mtime = parent_inode->i_ctime = CURRENT_TIME;
-		ret = btrfs_update_inode(trans, root, parent_inode);
+		parent_inode->i_mtime = parent_inode->i_ctime = inode->i_ctime
+				      = CURRENT_TIME;
+
+		ret = btrfs_update_inode(trans, root, inode);
+		if (!ret)
+			ret = btrfs_update_inode(trans, root, parent_inode);
 	}
 	return ret;
 }
Signature · 1 line
1.6.5.2
#1 lore raw
Quoted 11 lines · Miao Xie

the ctime of file has not been updated when I create a link for it.

Steps to reproduce:

 # touch file1
 # stat -c %Z file1
 1273592239
 # link flink1 file1
 # stat -c %Z file1
 1273592239             <-- have not been updated

This patch fix this problem.

Care to add a test to xfstests to check for this regression?

#2 lore raw

cc xfstests ml

Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Miao Xie

the ctime of file has not been updated when I create a link for it.

Steps to reproduce:

 # touch file1
 # stat -c %Z file1
 1273592239
 # link flink1 file1
 # stat -c %Z file1
 1273592239             <-- have not been updated
Quoted 3 lines
Quoted 1 line

This patch fix this problem.

Care to add a test to xfstests to check for this regression?

did it.

---

229 +73 -0
diff -urpN xfstests.orig/229 xfstests/229
@@ -0,0 +1,73 @@
+#! /bin/bash
+# FS QA Test No. 229
+#
+# Check ctime updated or not if file linked
+# See also http://marc.info/?l=linux-btrfs&m=127434439020230&w=2
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 FUJITSU LIMITED. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+_cleanup()
+{
+	if [ -a $TEST_DIR/ouch2 ]; then
+		rm -f $TEST_DIR/ouch2
+	fi
+	if [ -a $TEST_DIR/ouch ]; then
+		rm -f $TEST_DIR/ouch
+	fi
+}
+
+here=`pwd`
+
+# get standard environment, filters and checks
+. ./common.rc
+
+# real QA test starts here
+_supported_fs generic
+# only Linux supports fallocate
+_supported_os Linux
+
+# create a file and get its ctime
+touch $TEST_DIR/ouch
+ctime=`stat -c %Z $TEST_DIR/ouch`
+sleep 1
+
+# create a link to a file and get existing file's ctime 
+link $TEST_DIR/ouch $TEST_DIR/ouch2
+ctime2=`stat -c %Z $TEST_DIR/ouch`
+
+# check ctime updated
+if [ $ctime2 -le $ctime ]; then
+	echo "ctime: $ctime -> $ctime2 "
+	echo "Fatal error: ctime not updated after link"
+	_cleanup
+	exit 1
+fi
+
+_cleanup
+
+echo "Test over."
+# success, all done
+status=0
+exit
229.out +2 -0
diff -urpN xfstests.orig/229.out xfstests/229.out
@@ -0,0 +1,2 @@
+QA output created by 229
+Test over.
group +1 -0
diff -urpN xfstests.orig/group xfstests/group
@@ -342,3 +342,4 @@ deprecated
 226 auto enospc
 227 auto fsr
 228 rw auto prealloc quick
+229 auto

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw
In reply to Miao Xie

On Thu, May 20, 2010 at 12:22 AM, Miao Xie <[email protected]> wrote= :

Quoted 9 lines

the ctime of file has not been updated when I create a link for it.

Steps to reproduce:
=C2=A0# touch file1
=C2=A0# stat -c %Z file1
=C2=A01273592239
=C2=A0# link flink1 file1
=C2=A0# stat -c %Z file1
=C2=A01273592239 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <-- have n=

ot been updated

fs/btrfs/inode.c int btrfs_add_link(struct btrfs_trans_handle *=

Quoted 13 lines

This patch fix this problem.

---
=C2=A0fs/btrfs/inode.c | =C2=A0 =C2=A08 ++++++-- =C2=A01 files changed, 6 insertions(+), 2 deletions(-)

fs/btrfs/inode.c +0 -0
index a85b90c..5271887 100644
@@ -4218,8 +4218,12 @@ int btrfs_add_link(struct btrfs_trans_handle *=

trans,

Quoted 2 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0btrfs_i_size_w=

rite(parent_inode, parent_inode->i_size +

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 name_len * 2);

Quoted 1 line

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 parent_inode->i_mt=

ime =3D parent_inode->i_ctime =3D CURRENT_TIME;

Quoted 1 line

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D btrfs_upda=

te_inode(trans, root, parent_inode);

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 parent_inode->i_mt=

ime =3D parent_inode->i_ctime =3D inode->i_ctime

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D CURRENT_TI=
ME;

Quoted 2 lines

+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D btrfs_upda=

te_inode(trans, root, inode);

Quoted 2 lines

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!ret)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 ret =3D btrfs_update_inode(trans, root, parent_inode);

You only update parent inode if write to current inode fails?

Also should you be updating the ctime of parent inode even with link count of parent inode is not modified (btrfs always reports link count of 1 on directories)?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#4 lore raw
Quoted 24 lines · Mike Fedyk
Quoted 23 lines · Miao Xie

the ctime of file has not been updated when I create a link for it.

Steps to reproduce:

 # touch file1
 # stat -c %Z file1
 1273592239
 # link flink1 file1
 # stat -c %Z file1
 1273592239             <-- have not been updated

This patch fix this problem.

---
 fs/btrfs/inode.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
fs/btrfs/inode.c +0 -0
index a85b90c..5271887 100644
@@ -4218,8 +4218,12 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 
Quoted 12 lines

fs/btrfs/inode.c int btrfs_add_link(struct btrfs_trans_handle *trans, · line 4226

Quoted 10 lines
fs/btrfs/inode.c +6 -2
               btrfs_i_size_write(parent_inode, parent_inode->i_size +
                                  name_len * 2);
-               parent_inode->i_mtime = parent_inode->i_ctime = CURRENT_TIME;
-               ret = btrfs_update_inode(trans, root, parent_inode);
+               parent_inode->i_mtime = parent_inode->i_ctime = inode->i_ctime
+                                     = CURRENT_TIME;
+
+               ret = btrfs_update_inode(trans, root, inode);
+               if (!ret)
+                       ret = btrfs_update_inode(trans, root, parent_inode);

You only update parent inode if write to current inode fails?

I think if write to current inode fails, updating the parent inode is unnecessary, it is better to do rollback.

Quoted 4 lines

Also should you be updating the ctime of parent inode even with link count of parent inode is not modified (btrfs always reports link count of 1 on directories)?

the i_size of the parent inode is changed, so we must update the ctime of parent inode.

Thanks