Re: [PATCH v3 09/15] btrfs: add a BTRFS_FS_NEED_TRANS_COMMIT flag

[Anand Jain <anand.jain@oracle.com>]

On 19/10/2022 22:50, Josef Bacik wrote:
> Currently we are only using fs_info->pending_changes to indicate that we
> need a transaction commit.  The original users for this were removed
> years ago, so this is the only remaining reason to have this field.  Add
> a flag so we can remove this code.
> 
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Why did we miss my rb though some of the patches have no code-change, 
for example this patch.

Hm. Let's move on. Copying RB from v1.

Reviewed-by: Anand Jain <anand.jain@oracle.com>


> ---
>   fs/btrfs/fs.h          | 3 +++
>   fs/btrfs/super.c       | 3 ++-
>   fs/btrfs/sysfs.c       | 4 ++--
>   fs/btrfs/transaction.c | 2 ++
>   4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> index 2d06add70695..7b221d37ad0e 100644
> --- a/fs/btrfs/fs.h
> +++ b/fs/btrfs/fs.h
> @@ -90,6 +90,9 @@ enum {
>   	/* Indicate we have to finish a zone to do next allocation. */
>   	BTRFS_FS_NEED_ZONE_FINISH,
>   
> +	/* Indicate that we want to commit the transaction. */
> +	BTRFS_FS_NEED_TRANS_COMMIT,
> +
>   #if BITS_PER_LONG == 32
>   	/* Indicate if we have error/warn message printed on 32bit systems */
>   	BTRFS_FS_32BIT_ERROR,
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index daab56b6a582..7e9c1bff2fd6 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1534,7 +1534,8 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
>   			 * Exit unless we have some pending changes
>   			 * that need to go through commit
>   			 */
> -			if (fs_info->pending_changes == 0)
> +			if (!test_bit(BTRFS_FS_NEED_TRANS_COMMIT,
> +				      &fs_info->flags))
>   				return 0;
>   			/*
>   			 * A non-blocking test if the fs is frozen. We must not
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 0d98984af0e9..eb1a98991ec3 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -250,7 +250,7 @@ static ssize_t btrfs_feature_attr_store(struct kobject *kobj,
>   	/*
>   	 * We don't want to do full transaction commit from inside sysfs
>   	 */
> -	btrfs_set_pending(fs_info, COMMIT);
> +	set_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
>   	wake_up_process(fs_info->transaction_kthread);
>   
>   	return count;
> @@ -961,7 +961,7 @@ static ssize_t btrfs_label_store(struct kobject *kobj,
>   	/*
>   	 * We don't want to do full transaction commit from inside sysfs
>   	 */
> -	btrfs_set_pending(fs_info, COMMIT);
> +	set_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
>   	wake_up_process(fs_info->transaction_kthread);
>   
>   	return len;
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index bae77fb05e2b..7b6b68ab089a 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2104,6 +2104,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>   	ASSERT(refcount_read(&trans->use_count) == 1);
>   	btrfs_trans_state_lockdep_acquire(fs_info, BTRFS_LOCKDEP_TRANS_COMMIT_START);
>   
> +	clear_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
> +
>   	/* Stop the commit early if ->aborted is set */
>   	if (TRANS_ABORTED(cur_trans)) {
>   		ret = cur_trans->aborted;