Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 1 Nov 2020 08:09:29 +0100
From:   Andrea Righi <andrea.righi@canonical.com>
To:     Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc:     linux-ext4@vger.kernel.org, tytso@mit.edu, jack@suse.cz
Subject: Re: [PATCH 08/10] ext4: fix inode dirty check in case of fast commits
Message-ID: <20201101070929.GA3989@xps-13-7390>
References: <20201031200518.4178786-1-harshadshirwadkar@gmail.com>
 <20201031200518.4178786-9-harshadshirwadkar@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201031200518.4178786-9-harshadshirwadkar@gmail.com>
Precedence: bulk

On Sat, Oct 31, 2020 at 01:05:16PM -0700, Harshad Shirwadkar wrote:
> In case of fast commits, determine if the inode is dirty by checking
> if the inode is on fast commit list. This also helps us get rid of
> ext4_inode_info.i_fc_committed_subtid field.
> 
> Reported-by: Andrea Righi <andrea.righi@canonical.com>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

Tested and looks good to me. Thanks Harshad!

Tested-by: Andrea Righi <andrea.righi@canonical.com>

> ---
>  fs/ext4/ext4.h        | 3 ---
>  fs/ext4/fast_commit.c | 3 ---
>  fs/ext4/inode.c       | 3 +--
>  3 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 573db158382f..7222a9ba5d66 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1028,9 +1028,6 @@ struct ext4_inode_info {
>  					 * protected by sbi->s_fc_lock.
>  					 */
>  
> -	/* Fast commit subtid when this inode was committed */
> -	unsigned int i_fc_committed_subtid;
> -
>  	/* Start of lblk range that needs to be committed in this fast commit */
>  	ext4_lblk_t i_fc_lblk_start;
>  
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index b7b1fe6dbb24..4c0a3e858ea3 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -152,7 +152,6 @@ void ext4_fc_init_inode(struct inode *inode)
>  	INIT_LIST_HEAD(&ei->i_fc_list);
>  	init_waitqueue_head(&ei->i_fc_wait);
>  	atomic_set(&ei->i_fc_updates, 0);
> -	ei->i_fc_committed_subtid = 0;
>  }
>  
>  static void ext4_fc_wait_committing_inode(struct inode *inode)
> @@ -1026,8 +1025,6 @@ static int ext4_fc_perform_commit(journal_t *journal)
>  		if (ret)
>  			goto out;
>  		spin_lock(&sbi->s_fc_lock);
> -		EXT4_I(inode)->i_fc_committed_subtid =
> -			atomic_read(&sbi->s_fc_subtid);
>  	}
>  	spin_unlock(&sbi->s_fc_lock);
>  
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 7f6af784e74f..d36c3908272f 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3311,8 +3311,7 @@ static bool ext4_inode_datasync_dirty(struct inode *inode)
>  			EXT4_I(inode)->i_datasync_tid))
>  			return false;
>  		if (test_opt2(inode->i_sb, JOURNAL_FAST_COMMIT))
> -			return atomic_read(&EXT4_SB(inode->i_sb)->s_fc_subtid) <
> -				EXT4_I(inode)->i_fc_committed_subtid;
> +			return !list_empty(&EXT4_I(inode)->i_fc_list);
>  		return true;
>  	}
>  
> -- 
> 2.29.1.341.ge80a0c044ae-goog