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.f...@cn.fujitsu.com>

Good catch!

Reviewed-by: Miao Xie <miao...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to