[PATCH] Log parent inode if it is newer than the last commit

[PATCH] Log parent inode if it is newer than the last commit

[Itaru Kitayama]

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

Re: [PATCH] Log parent inode if it is newer than the last commit

[Josef Bacik]

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

Re: [PATCH] Log parent inode if it is newer than the last commit

[Chris Mason]

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

Re: [PATCH] Log parent inode if it is newer than the last commit

[Itaru Kitayama]

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

Re: [PATCH] Log parent inode if it is newer than the last commit

[Chris Mason]

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