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. Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/inode.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index a85b90c..5271887 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -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; } -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2010-May-20 08:33 UTC
Re: [PATCH 07/10] btrfs: fix wrong ctime when adding link
On Thu, May 20, 2010 at 03:22:30PM +0800, Miao Xie wrote:> 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? -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mike Fedyk
2010-May-20 09:01 UTC
Re: [PATCH 07/10] btrfs: fix wrong ctime when adding link
On Thu, May 20, 2010 at 12:22 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:> 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. > > Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> > --- > fs/btrfs/inode.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index a85b90c..5271887 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -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);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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
on 2010-5-20 17:01, Mike Fedyk wrote:> On Thu, May 20, 2010 at 12:22 AM, Miao Xie <miaox@cn.fujitsu.com> wrote: >> 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. >> >> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> >> --- >> fs/btrfs/inode.c | 8 ++++++-- >> 1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index a85b90c..5271887 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -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); > > 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.> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Shi Weihua
2010-May-24 06:37 UTC
Re: [PATCH 07/10] btrfs: fix wrong ctime when adding link
cc xfstests ml at 2010-5-20 16:33, Christoph Hellwig wrote:> On Thu, May 20, 2010 at 03:22:30PM +0800, Miao Xie wrote: >> 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?did it. Signed-off-by: Shi Weihua <shiwh@cn.fujitsu.com> --- diff -urpN xfstests.orig/229 xfstests/229 --- xfstests.orig/229 1970-01-01 08:00:00.000000000 +0800 +++ xfstests/229 2010-05-28 11:27:14.000000000 +0800 @@ -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 +owner=shiwh@cn.fujitsu.com + +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 diff -urpN xfstests.orig/229.out xfstests/229.out --- xfstests.orig/229.out 1970-01-01 08:00:00.000000000 +0800 +++ xfstests/229.out 2010-05-28 11:27:14.000000000 +0800 @@ -0,0 +1,2 @@ +QA output created by 229 +Test over. diff -urpN xfstests.orig/group xfstests/group --- xfstests.orig/group 2010-05-07 23:32:13.000000000 +0800 +++ xfstests/group 2010-05-28 11:27:02.000000000 +0800 @@ -342,3 +342,4 @@ deprecated 226 auto enospc 227 auto fsr 228 rw auto prealloc quick +229 auto -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html