Btrfs devel - Nov 2010 - [PATCH] Log parent inode if it is newer than the last commit

In the file sync path, file''s parent inode is logged if it is newer
than the last
commit. This patch checks also the last_trans field as well as generation.

As btrfs_log_inode updates the logged_trans field of parent dir''s
inode, tree-log
lookup operations are suppressed upon unlink.

The patch is for the latest btrfs-unstable tree.

Signed-off-by: Itaru Kitayama <kitayama@cl.bb4u.ne.jp>

---
 fs/btrfs/tree-log.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index a29f193..db63ae4 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3007,7 +3007,7 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle
*trans,
                if (root != BTRFS_I(inode)->root)
                        break;
 
-               if (BTRFS_I(inode)->generation >
+               if (max(BTRFS_I(inode)->generation,
BTRFS_I(inode)->last_trans) >
                    root->fs_info->last_trans_committed) {
                        ret = btrfs_log_inode(trans, root, inode, inode_only);
                        if (ret)
-- 
1.7.2.3
--
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 Sat, Nov 06, 2010 at 08:03:05PM +0900, Itaru Kitayama
wrote:
> 
> In the file sync path, file''s parent inode is logged if it is
newer than the last
> commit. This patch checks also the last_trans field as well as generation.
> 
> As btrfs_log_inode updates the logged_trans field of parent dir''s
inode, tree-log
> lookup operations are suppressed upon unlink.
> 
> The patch is for the latest btrfs-unstable tree.
> 
> Signed-off-by: Itaru Kitayama <kitayama@cl.bb4u.ne.jp>
> 
> ---
>  fs/btrfs/tree-log.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index a29f193..db63ae4 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3007,7 +3007,7 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle
*trans,
>                 if (root != BTRFS_I(inode)->root)
>                         break;
>  
> -               if (BTRFS_I(inode)->generation >
> +               if (max(BTRFS_I(inode)->generation,
BTRFS_I(inode)->last_trans) >
>                     root->fs_info->last_trans_committed) {
>                         ret = btrfs_log_inode(trans, root, inode,
inode_only);
>                         if (ret)
Looks good to me,

Reviewed-by: Josef Bacik <josef@redhat.com>

Thanks,

Josef
--
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

Excerpts from Itaru Kitayama''s message of 2010-11-06 07:03:05
-0400:
> 
> In the file sync path, file''s parent inode is logged if it is
newer than the last
> commit. This patch checks also the last_trans field as well as generation.
> 
> As btrfs_log_inode updates the logged_trans field of parent dir''s
inode, tree-log
> lookup operations are suppressed upon unlink.
Is there a specific test case you''re working on for this?  The idea
behind the logging code is that we use the backrefs in the inode to
recreate the inode in the directory if we crash.

This doesn''t work if the directory doesn''t exist when we come
back, so
the code logs the directory if it is newer than the last commit.

As long as the directory exists in the tree, we should be able to safely
continue replay without logging it again.

-chris
--
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

I''ve been puzzled why I see a flood of ENOENT returned upon rename
during Fedora 13 and 14''s GDM session.

The case is of course handled by your recent patch which is in upstream and
btrfs-unstable, but I thought
doing a log look-up always fail is inefficient so I checked the code path that
set inode''s logged_trans field.

Itaru

On Mon, 08 Nov 2010 08:50:21 -0500
Chris Mason <chris.mason@oracle.com> wrote:
> Excerpts from Itaru Kitayama''s message of 2010-11-06 07:03:05
-0400:
> > 
> > In the file sync path, file''s parent inode is logged if it is
newer than the last
> > commit. This patch checks also the last_trans field as well as
generation.
> > 
> > As btrfs_log_inode updates the logged_trans field of parent
dir''s inode, tree-log
> > lookup operations are suppressed upon unlink.
> 
> Is there a specific test case you''re working on for this?  The
idea
> behind the logging code is that we use the backrefs in the inode to
> recreate the inode in the directory if we crash.
> 
> This doesn''t work if the directory doesn''t exist when we
come back, so
> the code logs the directory if it is newer than the last commit.
> 
> As long as the directory exists in the tree, we should be able to safely
> continue replay without logging it again.
> 
> -chris
--
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

Excerpts from Itaru Kitayama''s message of 2010-11-08 09:27:51
-0500:
> 
> I''ve been puzzled why I see a flood of ENOENT returned upon rename
during Fedora 13 and 14''s GDM session.
> 
> The case is of course handled by your recent patch which is in upstream and
btrfs-unstable, but I thought
> doing a log look-up always fail is inefficient so I checked the code path
that set inode''s logged_trans field.
Oh ok, I thought you were trying to make sure it was relogged, so I read
this wrong.  You''re assuming that if it was ever logged we
don''t need to
log it again, which does look correct.

Thanks for the patch.

-chris
--
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