Re: [PATCH 11/16] btrfs: add a BTRFS_FS_NEED_TRANS_COMMIT flag

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

On 9/15/22 01:18, 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.
> 

The following #defines are unused. Why not remove them?

#define btrfs_test_pending(info, opt)  \
        test_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
#define btrfs_set_pending(info, opt)   \
        set_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)
#define btrfs_clear_pending(info, opt) \
        clear_bit(BTRFS_PENDING_##opt, &(info)->pending_changes)



Thanks, Anand





> Signed-off-by: Josef Bacik <josef@toxicpanda.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 c7eb21768b66..6181657bdc90 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 2865676b8327..63a639056882 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1547,7 +1547,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 4acff123fe66..6a7ab762a543 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;