| Message ID | 1428552523-3836-1-git-send-email-yangds.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, 09 Apr 2015 12:08:43 +0800, Dongsheng Yang wrote: > We need to fill inode when we found a node for it in delayed_nodes_tree. > But we did not fill the ->last_trans currently, it will cause the test > of xfstest/generic/311 fail. Scenario of the 311 is shown as below: > > Problem: > (1). test_fd = open(fname, O_RDWR|O_DIRECT) > (2). pwrite(test_fd, buf, 4096, 0) > (3). close(test_fd) > (4). drop_all_caches() <-------- "echo 3 > /proc/sys/vm/drop_caches" > (5). test_fd = open(fname, O_RDWR|O_DIRECT) > (6). fsync(test_fd); > <-------- we did not get the correct log entry for the file > Reason: > When we re-open this file in (5), we would find a node > in delayed_nodes_tree and fill the inode we are lookup with the > information. But the ->last_trans is not filled, then the fsync() > will check the ->last_trans and found it's 0 then say this inode > is already in our tree which is commited, not recording the extents > for it. > > Fix: > This patch fill the ->last_trans properly and set the > runtime_flags if needed in this situation. Then we can get the > log entries we expected after (6) and generic/311 passed. > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> Good catch! Reviewed-by: Miao Xie <miaoxie@huawei.com> > --- > fs/btrfs/delayed-inode.c | 2 ++ > fs/btrfs/inode.c | 21 ++++++++++++--------- > 2 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > index 82f0c7c..9e8b435 100644 > --- a/fs/btrfs/delayed-inode.c > +++ b/fs/btrfs/delayed-inode.c > @@ -1801,6 +1801,8 @@ int btrfs_fill_inode(struct inode *inode, u32 *rdev) > set_nlink(inode, btrfs_stack_inode_nlink(inode_item)); > inode_set_bytes(inode, btrfs_stack_inode_nbytes(inode_item)); > BTRFS_I(inode)->generation = btrfs_stack_inode_generation(inode_item); > + BTRFS_I(inode)->last_trans = btrfs_stack_inode_transid(inode_item); > + > inode->i_version = btrfs_stack_inode_sequence(inode_item); > inode->i_rdev = 0; > *rdev = btrfs_stack_inode_rdev(inode_item); > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index d2e732d..b132936 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -3628,25 +3628,28 @@ static void btrfs_read_locked_inode(struct inode *inode) > BTRFS_I(inode)->generation = btrfs_inode_generation(leaf, inode_item); > BTRFS_I(inode)->last_trans = btrfs_inode_transid(leaf, inode_item); > > + inode->i_version = btrfs_inode_sequence(leaf, inode_item); > + inode->i_generation = BTRFS_I(inode)->generation; > + inode->i_rdev = 0; > + rdev = btrfs_inode_rdev(leaf, inode_item); > + > + BTRFS_I(inode)->index_cnt = (u64)-1; > + BTRFS_I(inode)->flags = btrfs_inode_flags(leaf, inode_item); > + > +cache_index: > /* > * If we were modified in the current generation and evicted from memory > * and then re-read we need to do a full sync since we don't have any > * idea about which extents were modified before we were evicted from > * cache. > + * > + * This is required for both inode re-read from disk and delayed inode > + * in delayed_nodes_tree. > */ > if (BTRFS_I(inode)->last_trans == root->fs_info->generation) > set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, > &BTRFS_I(inode)->runtime_flags); > > - inode->i_version = btrfs_inode_sequence(leaf, inode_item); > - inode->i_generation = BTRFS_I(inode)->generation; > - inode->i_rdev = 0; > - rdev = btrfs_inode_rdev(leaf, inode_item); > - > - BTRFS_I(inode)->index_cnt = (u64)-1; > - BTRFS_I(inode)->flags = btrfs_inode_flags(leaf, inode_item); > - > -cache_index: > path->slots[0]++; > if (inode->i_nlink != 1 || > path->slots[0] >= btrfs_header_nritems(leaf)) > -- 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
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index 82f0c7c..9e8b435 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -1801,6 +1801,8 @@ int btrfs_fill_inode(struct inode *inode, u32 *rdev) set_nlink(inode, btrfs_stack_inode_nlink(inode_item)); inode_set_bytes(inode, btrfs_stack_inode_nbytes(inode_item)); BTRFS_I(inode)->generation = btrfs_stack_inode_generation(inode_item); + BTRFS_I(inode)->last_trans = btrfs_stack_inode_transid(inode_item); + inode->i_version = btrfs_stack_inode_sequence(inode_item); inode->i_rdev = 0; *rdev = btrfs_stack_inode_rdev(inode_item); diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d2e732d..b132936 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -3628,25 +3628,28 @@ static void btrfs_read_locked_inode(struct inode *inode) BTRFS_I(inode)->generation = btrfs_inode_generation(leaf, inode_item); BTRFS_I(inode)->last_trans = btrfs_inode_transid(leaf, inode_item); + inode->i_version = btrfs_inode_sequence(leaf, inode_item); + inode->i_generation = BTRFS_I(inode)->generation; + inode->i_rdev = 0; + rdev = btrfs_inode_rdev(leaf, inode_item); + + BTRFS_I(inode)->index_cnt = (u64)-1; + BTRFS_I(inode)->flags = btrfs_inode_flags(leaf, inode_item); + +cache_index: /* * If we were modified in the current generation and evicted from memory * and then re-read we need to do a full sync since we don't have any * idea about which extents were modified before we were evicted from * cache. + * + * This is required for both inode re-read from disk and delayed inode + * in delayed_nodes_tree. */ if (BTRFS_I(inode)->last_trans == root->fs_info->generation) set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, &BTRFS_I(inode)->runtime_flags); - inode->i_version = btrfs_inode_sequence(leaf, inode_item); - inode->i_generation = BTRFS_I(inode)->generation; - inode->i_rdev = 0; - rdev = btrfs_inode_rdev(leaf, inode_item); - - BTRFS_I(inode)->index_cnt = (u64)-1; - BTRFS_I(inode)->flags = btrfs_inode_flags(leaf, inode_item); - -cache_index: path->slots[0]++; if (inode->i_nlink != 1 || path->slots[0] >= btrfs_header_nritems(leaf))
We need to fill inode when we found a node for it in delayed_nodes_tree. But we did not fill the ->last_trans currently, it will cause the test of xfstest/generic/311 fail. Scenario of the 311 is shown as below: Problem: (1). test_fd = open(fname, O_RDWR|O_DIRECT) (2). pwrite(test_fd, buf, 4096, 0) (3). close(test_fd) (4). drop_all_caches() <-------- "echo 3 > /proc/sys/vm/drop_caches" (5). test_fd = open(fname, O_RDWR|O_DIRECT) (6). fsync(test_fd); <-------- we did not get the correct log entry for the file Reason: When we re-open this file in (5), we would find a node in delayed_nodes_tree and fill the inode we are lookup with the information. But the ->last_trans is not filled, then the fsync() will check the ->last_trans and found it's 0 then say this inode is already in our tree which is commited, not recording the extents for it. Fix: This patch fill the ->last_trans properly and set the runtime_flags if needed in this situation. Then we can get the log entries we expected after (6) and generic/311 passed. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/btrfs/delayed-inode.c | 2 ++ fs/btrfs/inode.c | 21 ++++++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-)