* [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h
2026-09-24 5:58 [PATCH v2 0/3] btrfs: __TRANS_* flags cleanup Qu Wenruo
@ 2026-09-24 5:58 ` Qu Wenruo
2026-09-28 18:27 ` Filipe Manana
2026-09-24 5:58 ` [PATCH v2 2/3] btrfs: remove __TRANS_FREEZABLE Qu Wenruo
2026-09-24 5:58 ` [PATCH v2 3/3] btrfs: remove __TRANS_* flags Qu Wenruo
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2026-09-24 5:58 UTC (permalink / raw)
To: linux-btrfs
Among all those flags, only __TRANS_DUMMY is used outside of
transaction.[ch], and there are only two places using it:
- find_parent_nodes()
Introduce a helper, btrfs_trans_is_dummy(), for this call site.
- btrfs_init_dummy_trans()
Move the function into transaction.[ch], and hide it behind
CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
With the above changes, we can hide __TRANS_* and TRANS_* flags inside
transaction.c. This allows us to modify those flags in the future
without causing any changes to existing callers.
After the flags relocation, now we can also hide __TRANS_DUMMY behind
CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/backref.c | 2 +-
fs/btrfs/tests/btrfs-tests.c | 9 ---------
fs/btrfs/tests/btrfs-tests.h | 2 --
fs/btrfs/transaction.c | 36 ++++++++++++++++++++++++++++++++++++
fs/btrfs/transaction.h | 29 +++++++++++------------------
5 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 1be632c742bd..939511629ecc 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1432,7 +1432,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
goto out;
}
- if (ctx->trans && likely(ctx->trans->type != __TRANS_DUMMY) &&
+ if (ctx->trans && likely(!btrfs_trans_is_dummy(ctx->trans)) &&
ctx->time_seq != BTRFS_SEQ_LAST) {
/*
* We have a specific time_seq we care about and trans which
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 6287d940323d..cebc9a17b94d 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -247,15 +247,6 @@ void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_
spin_lock_init(&trans->delayed_refs.lock);
}
-void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info)
-{
- memset(trans, 0, sizeof(*trans));
- trans->transid = 1;
- trans->type = __TRANS_DUMMY;
- trans->fs_info = fs_info;
-}
-
int btrfs_run_sanity_tests(void)
{
int ret, i;
diff --git a/fs/btrfs/tests/btrfs-tests.h b/fs/btrfs/tests/btrfs-tests.h
index cea58fe84a6d..a16855138105 100644
--- a/fs/btrfs/tests/btrfs-tests.h
+++ b/fs/btrfs/tests/btrfs-tests.h
@@ -59,8 +59,6 @@ btrfs_alloc_dummy_block_group(struct btrfs_fs_info *fs_info, unsigned long lengt
void btrfs_free_dummy_block_group(struct btrfs_block_group *cache);
DEFINE_FREE(btrfs_free_dummy_block_group, struct btrfs_block_group *,
btrfs_free_dummy_block_group(_T));
-void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info);
void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_fs_info *fs_info);
struct btrfs_device *btrfs_alloc_dummy_device(struct btrfs_fs_info *fs_info);
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index ca114235bbe1..4181f3897a30 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -38,6 +38,26 @@
static struct kmem_cache *btrfs_trans_handle_cachep;
+enum {
+ ENUM_BIT(__TRANS_FREEZABLE),
+ ENUM_BIT(__TRANS_START),
+ ENUM_BIT(__TRANS_ATTACH),
+ ENUM_BIT(__TRANS_JOIN),
+ ENUM_BIT(__TRANS_JOIN_NOLOCK),
+#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
+ ENUM_BIT(__TRANS_DUMMY),
+#endif
+ ENUM_BIT(__TRANS_JOIN_NOSTART),
+};
+
+#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
+#define TRANS_ATTACH (__TRANS_ATTACH)
+#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
+#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
+#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
+
+#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
+
/*
* Transaction states and transitions
*
@@ -139,6 +159,22 @@ static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
__TRANS_JOIN_NOSTART),
};
+#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
+bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
+{
+ return trans->type == __TRANS_DUMMY;
+}
+
+void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
+ struct btrfs_fs_info *fs_info)
+{
+ memset(trans, 0, sizeof(*trans));
+ trans->transid = 1;
+ trans->type = __TRANS_DUMMY;
+ trans->fs_info = fs_info;
+}
+#endif
+
void btrfs_put_transaction(struct btrfs_transaction *transaction)
{
if (refcount_dec_and_test(&transaction->use_count)) {
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 89153cd22596..9eb92b1ad54d 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -119,24 +119,6 @@ struct btrfs_transaction {
wait_queue_head_t pending_wait;
};
-enum {
- ENUM_BIT(__TRANS_FREEZABLE),
- ENUM_BIT(__TRANS_START),
- ENUM_BIT(__TRANS_ATTACH),
- ENUM_BIT(__TRANS_JOIN),
- ENUM_BIT(__TRANS_JOIN_NOLOCK),
- ENUM_BIT(__TRANS_DUMMY),
- ENUM_BIT(__TRANS_JOIN_NOSTART),
-};
-
-#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
-#define TRANS_ATTACH (__TRANS_ATTACH)
-#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
-#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
-#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
-
-#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
-
/*
* Number of extent buffers a transaction handle tracks for writeback
* inhibition. The CLOCK reference bits pack into a u32 so this must not exceed
@@ -305,6 +287,17 @@ do { \
__LINE__, __error); \
} while (0)
+#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
+bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans);
+void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
+ struct btrfs_fs_info *fs_info);
+#else
+static inline bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
+{
+ return false;
+}
+#endif
+
int btrfs_end_transaction(struct btrfs_trans_handle *trans);
struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
unsigned int num_items);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h
2026-09-24 5:58 ` [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h Qu Wenruo
@ 2026-09-28 18:27 ` Filipe Manana
2026-09-28 21:59 ` Qu Wenruo
0 siblings, 1 reply; 8+ messages in thread
From: Filipe Manana @ 2026-09-28 18:27 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Sep 24, 2026 at 7:00 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Among all those flags, only __TRANS_DUMMY is used outside of
> transaction.[ch], and there are only two places using it:
>
> - find_parent_nodes()
> Introduce a helper, btrfs_trans_is_dummy(), for this call site.
>
> - btrfs_init_dummy_trans()
> Move the function into transaction.[ch], and hide it behind
> CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
>
> With the above changes, we can hide __TRANS_* and TRANS_* flags inside
> transaction.c. This allows us to modify those flags in the future
> without causing any changes to existing callers.
>
> After the flags relocation, now we can also hide __TRANS_DUMMY behind
> CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/backref.c | 2 +-
> fs/btrfs/tests/btrfs-tests.c | 9 ---------
> fs/btrfs/tests/btrfs-tests.h | 2 --
> fs/btrfs/transaction.c | 36 ++++++++++++++++++++++++++++++++++++
> fs/btrfs/transaction.h | 29 +++++++++++------------------
> 5 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 1be632c742bd..939511629ecc 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1432,7 +1432,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
> goto out;
> }
>
> - if (ctx->trans && likely(ctx->trans->type != __TRANS_DUMMY) &&
> + if (ctx->trans && likely(!btrfs_trans_is_dummy(ctx->trans)) &&
> ctx->time_seq != BTRFS_SEQ_LAST) {
> /*
> * We have a specific time_seq we care about and trans which
> diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
> index 6287d940323d..cebc9a17b94d 100644
> --- a/fs/btrfs/tests/btrfs-tests.c
> +++ b/fs/btrfs/tests/btrfs-tests.c
> @@ -247,15 +247,6 @@ void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_
> spin_lock_init(&trans->delayed_refs.lock);
> }
>
> -void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info)
> -{
> - memset(trans, 0, sizeof(*trans));
> - trans->transid = 1;
> - trans->type = __TRANS_DUMMY;
> - trans->fs_info = fs_info;
> -}
> -
> int btrfs_run_sanity_tests(void)
> {
> int ret, i;
> diff --git a/fs/btrfs/tests/btrfs-tests.h b/fs/btrfs/tests/btrfs-tests.h
> index cea58fe84a6d..a16855138105 100644
> --- a/fs/btrfs/tests/btrfs-tests.h
> +++ b/fs/btrfs/tests/btrfs-tests.h
> @@ -59,8 +59,6 @@ btrfs_alloc_dummy_block_group(struct btrfs_fs_info *fs_info, unsigned long lengt
> void btrfs_free_dummy_block_group(struct btrfs_block_group *cache);
> DEFINE_FREE(btrfs_free_dummy_block_group, struct btrfs_block_group *,
> btrfs_free_dummy_block_group(_T));
> -void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info);
> void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_fs_info *fs_info);
> struct btrfs_device *btrfs_alloc_dummy_device(struct btrfs_fs_info *fs_info);
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ca114235bbe1..4181f3897a30 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -38,6 +38,26 @@
>
> static struct kmem_cache *btrfs_trans_handle_cachep;
>
> +enum {
> + ENUM_BIT(__TRANS_FREEZABLE),
> + ENUM_BIT(__TRANS_START),
> + ENUM_BIT(__TRANS_ATTACH),
> + ENUM_BIT(__TRANS_JOIN),
> + ENUM_BIT(__TRANS_JOIN_NOLOCK),
> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> + ENUM_BIT(__TRANS_DUMMY),
> +#endif
> + ENUM_BIT(__TRANS_JOIN_NOSTART),
> +};
> +
> +#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
> +#define TRANS_ATTACH (__TRANS_ATTACH)
> +#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
> +#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
> +#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
> +
> +#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
> +
> /*
> * Transaction states and transitions
> *
> @@ -139,6 +159,22 @@ static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
> __TRANS_JOIN_NOSTART),
> };
>
> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> +bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
> +{
So most of our exported transaction functions have names like:
btrfs_end_transaction()
btrfs_commit_transaction()
btrfs_start_transaction()
btrfs_abort_transaction()
etc
So I would suggest, for consistency, naming it as: btrfs_is_dummy_transaction()
> + return trans->type == __TRANS_DUMMY;
> +}
> +
> +void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info)
> +{
And here; btrfs_init_dummy_transaction()
Otherwise:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> + memset(trans, 0, sizeof(*trans));
> + trans->transid = 1;
> + trans->type = __TRANS_DUMMY;
> + trans->fs_info = fs_info;
> +}
> +#endif
> +
> void btrfs_put_transaction(struct btrfs_transaction *transaction)
> {
> if (refcount_dec_and_test(&transaction->use_count)) {
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 89153cd22596..9eb92b1ad54d 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -119,24 +119,6 @@ struct btrfs_transaction {
> wait_queue_head_t pending_wait;
> };
>
> -enum {
> - ENUM_BIT(__TRANS_FREEZABLE),
> - ENUM_BIT(__TRANS_START),
> - ENUM_BIT(__TRANS_ATTACH),
> - ENUM_BIT(__TRANS_JOIN),
> - ENUM_BIT(__TRANS_JOIN_NOLOCK),
> - ENUM_BIT(__TRANS_DUMMY),
> - ENUM_BIT(__TRANS_JOIN_NOSTART),
> -};
> -
> -#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
> -#define TRANS_ATTACH (__TRANS_ATTACH)
> -#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
> -#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
> -#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
> -
> -#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
> -
> /*
> * Number of extent buffers a transaction handle tracks for writeback
> * inhibition. The CLOCK reference bits pack into a u32 so this must not exceed
> @@ -305,6 +287,17 @@ do { \
> __LINE__, __error); \
> } while (0)
>
> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> +bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans);
> +void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info);
> +#else
> +static inline bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
> +{
> + return false;
> +}
> +#endif
> +
> int btrfs_end_transaction(struct btrfs_trans_handle *trans);
> struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
> unsigned int num_items);
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h
2026-09-28 18:27 ` Filipe Manana
@ 2026-09-28 21:59 ` Qu Wenruo
0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2026-09-28 21:59 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs
在 2026/9/29 03:57, Filipe Manana 写道:
> On Thu, Sep 24, 2026 at 7:00 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> Among all those flags, only __TRANS_DUMMY is used outside of
>> transaction.[ch], and there are only two places using it:
>>
>> - find_parent_nodes()
>> Introduce a helper, btrfs_trans_is_dummy(), for this call site.
>>
>> - btrfs_init_dummy_trans()
>> Move the function into transaction.[ch], and hide it behind
>> CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
>>
>> With the above changes, we can hide __TRANS_* and TRANS_* flags inside
>> transaction.c. This allows us to modify those flags in the future
>> without causing any changes to existing callers.
>>
>> After the flags relocation, now we can also hide __TRANS_DUMMY behind
>> CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/backref.c | 2 +-
>> fs/btrfs/tests/btrfs-tests.c | 9 ---------
>> fs/btrfs/tests/btrfs-tests.h | 2 --
>> fs/btrfs/transaction.c | 36 ++++++++++++++++++++++++++++++++++++
>> fs/btrfs/transaction.h | 29 +++++++++++------------------
>> 5 files changed, 48 insertions(+), 30 deletions(-)
>>
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 1be632c742bd..939511629ecc 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -1432,7 +1432,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
>> goto out;
>> }
>>
>> - if (ctx->trans && likely(ctx->trans->type != __TRANS_DUMMY) &&
>> + if (ctx->trans && likely(!btrfs_trans_is_dummy(ctx->trans)) &&
>> ctx->time_seq != BTRFS_SEQ_LAST) {
>> /*
>> * We have a specific time_seq we care about and trans which
>> diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
>> index 6287d940323d..cebc9a17b94d 100644
>> --- a/fs/btrfs/tests/btrfs-tests.c
>> +++ b/fs/btrfs/tests/btrfs-tests.c
>> @@ -247,15 +247,6 @@ void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_
>> spin_lock_init(&trans->delayed_refs.lock);
>> }
>>
>> -void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
>> - struct btrfs_fs_info *fs_info)
>> -{
>> - memset(trans, 0, sizeof(*trans));
>> - trans->transid = 1;
>> - trans->type = __TRANS_DUMMY;
>> - trans->fs_info = fs_info;
>> -}
>> -
>> int btrfs_run_sanity_tests(void)
>> {
>> int ret, i;
>> diff --git a/fs/btrfs/tests/btrfs-tests.h b/fs/btrfs/tests/btrfs-tests.h
>> index cea58fe84a6d..a16855138105 100644
>> --- a/fs/btrfs/tests/btrfs-tests.h
>> +++ b/fs/btrfs/tests/btrfs-tests.h
>> @@ -59,8 +59,6 @@ btrfs_alloc_dummy_block_group(struct btrfs_fs_info *fs_info, unsigned long lengt
>> void btrfs_free_dummy_block_group(struct btrfs_block_group *cache);
>> DEFINE_FREE(btrfs_free_dummy_block_group, struct btrfs_block_group *,
>> btrfs_free_dummy_block_group(_T));
>> -void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
>> - struct btrfs_fs_info *fs_info);
>> void btrfs_init_dummy_transaction(struct btrfs_transaction *trans, struct btrfs_fs_info *fs_info);
>> struct btrfs_device *btrfs_alloc_dummy_device(struct btrfs_fs_info *fs_info);
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ca114235bbe1..4181f3897a30 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -38,6 +38,26 @@
>>
>> static struct kmem_cache *btrfs_trans_handle_cachep;
>>
>> +enum {
>> + ENUM_BIT(__TRANS_FREEZABLE),
>> + ENUM_BIT(__TRANS_START),
>> + ENUM_BIT(__TRANS_ATTACH),
>> + ENUM_BIT(__TRANS_JOIN),
>> + ENUM_BIT(__TRANS_JOIN_NOLOCK),
>> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>> + ENUM_BIT(__TRANS_DUMMY),
>> +#endif
>> + ENUM_BIT(__TRANS_JOIN_NOSTART),
>> +};
>> +
>> +#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
>> +#define TRANS_ATTACH (__TRANS_ATTACH)
>> +#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
>> +#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
>> +#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
>> +
>> +#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
>> +
>> /*
>> * Transaction states and transitions
>> *
>> @@ -139,6 +159,22 @@ static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
>> __TRANS_JOIN_NOSTART),
>> };
>>
>> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>> +bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
>> +{
>
> So most of our exported transaction functions have names like:
>
> btrfs_end_transaction()
> btrfs_commit_transaction()
> btrfs_start_transaction()
> btrfs_abort_transaction()
> etc
>
> So I would suggest, for consistency, naming it as: btrfs_is_dummy_transaction()
That sounds good.
>
>> + return trans->type == __TRANS_DUMMY;
>> +}
>> +
>> +void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
>> + struct btrfs_fs_info *fs_info)
>> +{
>
> And here; btrfs_init_dummy_transaction()
Unfortunately we already have one in the selftests with the same name,
but using btrfs_transaction as the first parameter.
Thus I'm afraid I have to keep this name.
Thanks,
Qu
>
> Otherwise:
>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> Thanks.
>
>> + memset(trans, 0, sizeof(*trans));
>> + trans->transid = 1;
>> + trans->type = __TRANS_DUMMY;
>> + trans->fs_info = fs_info;
>> +}
>> +#endif
>> +
>> void btrfs_put_transaction(struct btrfs_transaction *transaction)
>> {
>> if (refcount_dec_and_test(&transaction->use_count)) {
>> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
>> index 89153cd22596..9eb92b1ad54d 100644
>> --- a/fs/btrfs/transaction.h
>> +++ b/fs/btrfs/transaction.h
>> @@ -119,24 +119,6 @@ struct btrfs_transaction {
>> wait_queue_head_t pending_wait;
>> };
>>
>> -enum {
>> - ENUM_BIT(__TRANS_FREEZABLE),
>> - ENUM_BIT(__TRANS_START),
>> - ENUM_BIT(__TRANS_ATTACH),
>> - ENUM_BIT(__TRANS_JOIN),
>> - ENUM_BIT(__TRANS_JOIN_NOLOCK),
>> - ENUM_BIT(__TRANS_DUMMY),
>> - ENUM_BIT(__TRANS_JOIN_NOSTART),
>> -};
>> -
>> -#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
>> -#define TRANS_ATTACH (__TRANS_ATTACH)
>> -#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
>> -#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
>> -#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
>> -
>> -#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
>> -
>> /*
>> * Number of extent buffers a transaction handle tracks for writeback
>> * inhibition. The CLOCK reference bits pack into a u32 so this must not exceed
>> @@ -305,6 +287,17 @@ do { \
>> __LINE__, __error); \
>> } while (0)
>>
>> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>> +bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans);
>> +void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
>> + struct btrfs_fs_info *fs_info);
>> +#else
>> +static inline bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
>> +{
>> + return false;
>> +}
>> +#endif
>> +
>> int btrfs_end_transaction(struct btrfs_trans_handle *trans);
>> struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
>> unsigned int num_items);
>> --
>> 2.55.0
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] btrfs: remove __TRANS_FREEZABLE
2026-09-24 5:58 [PATCH v2 0/3] btrfs: __TRANS_* flags cleanup Qu Wenruo
2026-09-24 5:58 ` [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h Qu Wenruo
@ 2026-09-24 5:58 ` Qu Wenruo
2026-09-28 18:32 ` Filipe Manana
2026-09-24 5:58 ` [PATCH v2 3/3] btrfs: remove __TRANS_* flags Qu Wenruo
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2026-09-24 5:58 UTC (permalink / raw)
To: linux-btrfs
Inside transaction.c most TRANS_* flags are just a single bit, but there
are 2 exceptions:
- TRANS_START
Which is (__TRANS_START | __TRANS_FREEZABLE)
- TRANS_JOIN
Which is (__TRANS_JOIN | __TRANS_FREEZABLE)
The extra __TRANS_FREEZABLE flag indicates that those operations need
to acquire sb intwrite lock to handle fs freezing.
However since there are only two operations requiring sb intwrite lock,
there is no need to introduce a dedicated flag for it, we can introduce
a new TRANS_SB_INTWRITER_MASK to cover the only two cases, then use that
new mask to determine whether the type requires sb intwrite lock.
This makes all TRANS_* flags a single bit.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/transaction.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 4181f3897a30..d73c8723ed70 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -39,7 +39,6 @@
static struct kmem_cache *btrfs_trans_handle_cachep;
enum {
- ENUM_BIT(__TRANS_FREEZABLE),
ENUM_BIT(__TRANS_START),
ENUM_BIT(__TRANS_ATTACH),
ENUM_BIT(__TRANS_JOIN),
@@ -50,12 +49,15 @@ enum {
ENUM_BIT(__TRANS_JOIN_NOSTART),
};
-#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
+#define TRANS_START (__TRANS_START)
#define TRANS_ATTACH (__TRANS_ATTACH)
-#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
+#define TRANS_JOIN (__TRANS_JOIN)
#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
+/* Those types need to hold sb intwrite lock. */
+#define TRANS_SB_INTWRITER_MASK (__TRANS_START | __TRANS_JOIN)
+
#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
/*
@@ -746,6 +748,8 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
}
/*
+ * Only TRANS_START and TRANS_JOIN require sb intwrite lock.
+ *
* If we are JOIN_NOLOCK we're already committing a transaction and
* waiting on this guy, so we don't need to do the sb_start_intwrite
* because we're already holding a ref. We need this because we could
@@ -755,7 +759,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
* If we are ATTACH, it means we just want to catch the current
* transaction and commit it, so we needn't do sb_start_intwrite().
*/
- if (type & __TRANS_FREEZABLE)
+ if (type & TRANS_SB_INTWRITER_MASK)
sb_start_intwrite(fs_info->sb);
if (may_wait_transaction(fs_info, type))
@@ -857,7 +861,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
return h;
join_fail:
- if (type & __TRANS_FREEZABLE)
+ if (type & TRANS_SB_INTWRITER_MASK)
sb_end_intwrite(fs_info->sb);
kmem_cache_free(btrfs_trans_handle_cachep, h);
alloc_fail:
@@ -1138,7 +1142,7 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
btrfs_trans_release_chunk_metadata(trans);
- if (trans->type & __TRANS_FREEZABLE)
+ if (trans->type & TRANS_SB_INTWRITER_MASK)
sb_end_intwrite(info->sb);
/*
@@ -2150,7 +2154,7 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans, int err)
fs_info->running_transaction = NULL;
spin_unlock(&fs_info->trans_lock);
- if (trans->type & __TRANS_FREEZABLE)
+ if (trans->type & TRANS_SB_INTWRITER_MASK)
sb_end_intwrite(fs_info->sb);
btrfs_put_transaction(cur_trans);
btrfs_put_transaction(cur_trans);
@@ -2682,7 +2686,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
btrfs_put_transaction(cur_trans);
btrfs_put_transaction(cur_trans);
- if (trans->type & __TRANS_FREEZABLE)
+ if (trans->type & TRANS_SB_INTWRITER_MASK)
sb_end_intwrite(fs_info->sb);
btrfs_scrub_continue(fs_info);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] btrfs: remove __TRANS_FREEZABLE
2026-09-24 5:58 ` [PATCH v2 2/3] btrfs: remove __TRANS_FREEZABLE Qu Wenruo
@ 2026-09-28 18:32 ` Filipe Manana
0 siblings, 0 replies; 8+ messages in thread
From: Filipe Manana @ 2026-09-28 18:32 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Sep 24, 2026 at 7:01 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Inside transaction.c most TRANS_* flags are just a single bit, but there
> are 2 exceptions:
>
> - TRANS_START
> Which is (__TRANS_START | __TRANS_FREEZABLE)
>
> - TRANS_JOIN
> Which is (__TRANS_JOIN | __TRANS_FREEZABLE)
>
> The extra __TRANS_FREEZABLE flag indicates that those operations need
> to acquire sb intwrite lock to handle fs freezing.
>
> However since there are only two operations requiring sb intwrite lock,
> there is no need to introduce a dedicated flag for it, we can introduce
> a new TRANS_SB_INTWRITER_MASK to cover the only two cases, then use that
> new mask to determine whether the type requires sb intwrite lock.
>
> This makes all TRANS_* flags a single bit.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/transaction.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 4181f3897a30..d73c8723ed70 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -39,7 +39,6 @@
> static struct kmem_cache *btrfs_trans_handle_cachep;
>
> enum {
> - ENUM_BIT(__TRANS_FREEZABLE),
> ENUM_BIT(__TRANS_START),
> ENUM_BIT(__TRANS_ATTACH),
> ENUM_BIT(__TRANS_JOIN),
> @@ -50,12 +49,15 @@ enum {
> ENUM_BIT(__TRANS_JOIN_NOSTART),
> };
>
> -#define TRANS_START (__TRANS_START | __TRANS_FREEZABLE)
> +#define TRANS_START (__TRANS_START)
> #define TRANS_ATTACH (__TRANS_ATTACH)
> -#define TRANS_JOIN (__TRANS_JOIN | __TRANS_FREEZABLE)
> +#define TRANS_JOIN (__TRANS_JOIN)
So after this change, can't we rename the enum members to remove the
__ prefix and get rid of these #defines?
Since none of them are ORed combinations anymore...
Anyway:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> #define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
> #define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
>
> +/* Those types need to hold sb intwrite lock. */
> +#define TRANS_SB_INTWRITER_MASK (__TRANS_START | __TRANS_JOIN)
> +
> #define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
>
> /*
> @@ -746,6 +748,8 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
> }
>
> /*
> + * Only TRANS_START and TRANS_JOIN require sb intwrite lock.
> + *
> * If we are JOIN_NOLOCK we're already committing a transaction and
> * waiting on this guy, so we don't need to do the sb_start_intwrite
> * because we're already holding a ref. We need this because we could
> @@ -755,7 +759,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
> * If we are ATTACH, it means we just want to catch the current
> * transaction and commit it, so we needn't do sb_start_intwrite().
> */
> - if (type & __TRANS_FREEZABLE)
> + if (type & TRANS_SB_INTWRITER_MASK)
> sb_start_intwrite(fs_info->sb);
>
> if (may_wait_transaction(fs_info, type))
> @@ -857,7 +861,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
> return h;
>
> join_fail:
> - if (type & __TRANS_FREEZABLE)
> + if (type & TRANS_SB_INTWRITER_MASK)
> sb_end_intwrite(fs_info->sb);
> kmem_cache_free(btrfs_trans_handle_cachep, h);
> alloc_fail:
> @@ -1138,7 +1142,7 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>
> btrfs_trans_release_chunk_metadata(trans);
>
> - if (trans->type & __TRANS_FREEZABLE)
> + if (trans->type & TRANS_SB_INTWRITER_MASK)
> sb_end_intwrite(info->sb);
>
> /*
> @@ -2150,7 +2154,7 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans, int err)
> fs_info->running_transaction = NULL;
> spin_unlock(&fs_info->trans_lock);
>
> - if (trans->type & __TRANS_FREEZABLE)
> + if (trans->type & TRANS_SB_INTWRITER_MASK)
> sb_end_intwrite(fs_info->sb);
> btrfs_put_transaction(cur_trans);
> btrfs_put_transaction(cur_trans);
> @@ -2682,7 +2686,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> btrfs_put_transaction(cur_trans);
> btrfs_put_transaction(cur_trans);
>
> - if (trans->type & __TRANS_FREEZABLE)
> + if (trans->type & TRANS_SB_INTWRITER_MASK)
> sb_end_intwrite(fs_info->sb);
>
> btrfs_scrub_continue(fs_info);
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] btrfs: remove __TRANS_* flags
2026-09-24 5:58 [PATCH v2 0/3] btrfs: __TRANS_* flags cleanup Qu Wenruo
2026-09-24 5:58 ` [PATCH v2 1/3] btrfs: move __TRANS_* and TRANS_* flags out of transaction.h Qu Wenruo
2026-09-24 5:58 ` [PATCH v2 2/3] btrfs: remove __TRANS_FREEZABLE Qu Wenruo
@ 2026-09-24 5:58 ` Qu Wenruo
2026-09-28 18:34 ` Filipe Manana
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2026-09-24 5:58 UTC (permalink / raw)
To: linux-btrfs
After patch "btrfs: remove __TRANS_FREEZABLE", each TRANS_* flag is just
the corresponding single-bit __TRANS_* flag.
There is no need to split __TRANS_* and TRANS_* flags, just remove
__TRANS_* flags and use TRANS_* flags instead.
Since every TRANS_* flag is a single bit, do extra cleanups:
- Add an ASSERT() in start_transaction()
To make sure there is only a single bit set in @type
- Rename TRANS_EXTWRITERS to TRANS_EXTWRITERS_MASK
To follow the naming scheme that a multi-bit value has the _MASK
suffix.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/transaction.c | 77 ++++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 40 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index d73c8723ed70..2532b7b27f9a 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -39,26 +39,20 @@
static struct kmem_cache *btrfs_trans_handle_cachep;
enum {
- ENUM_BIT(__TRANS_START),
- ENUM_BIT(__TRANS_ATTACH),
- ENUM_BIT(__TRANS_JOIN),
- ENUM_BIT(__TRANS_JOIN_NOLOCK),
+ ENUM_BIT(TRANS_START),
+ ENUM_BIT(TRANS_ATTACH),
+ ENUM_BIT(TRANS_JOIN),
+ ENUM_BIT(TRANS_JOIN_NOLOCK),
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
- ENUM_BIT(__TRANS_DUMMY),
+ ENUM_BIT(TRANS_DUMMY),
#endif
- ENUM_BIT(__TRANS_JOIN_NOSTART),
+ ENUM_BIT(TRANS_JOIN_NOSTART),
};
-#define TRANS_START (__TRANS_START)
-#define TRANS_ATTACH (__TRANS_ATTACH)
-#define TRANS_JOIN (__TRANS_JOIN)
-#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
-#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
-
/* Those types need to hold sb intwrite lock. */
-#define TRANS_SB_INTWRITER_MASK (__TRANS_START | __TRANS_JOIN)
+#define TRANS_SB_INTWRITER_MASK (TRANS_START | TRANS_JOIN)
-#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
+#define TRANS_EXTWRITERS_MASK (TRANS_START | TRANS_ATTACH)
/*
* Transaction states and transitions
@@ -139,32 +133,32 @@ enum {
static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
[TRANS_STATE_RUNNING] = 0U,
[TRANS_STATE_COMMIT_PREP] = 0U,
- [TRANS_STATE_COMMIT_START] = (__TRANS_START | __TRANS_ATTACH),
- [TRANS_STATE_COMMIT_DOING] = (__TRANS_START |
- __TRANS_ATTACH |
- __TRANS_JOIN |
- __TRANS_JOIN_NOSTART),
- [TRANS_STATE_UNBLOCKED] = (__TRANS_START |
- __TRANS_ATTACH |
- __TRANS_JOIN |
- __TRANS_JOIN_NOLOCK |
- __TRANS_JOIN_NOSTART),
- [TRANS_STATE_SUPER_COMMITTED] = (__TRANS_START |
- __TRANS_ATTACH |
- __TRANS_JOIN |
- __TRANS_JOIN_NOLOCK |
- __TRANS_JOIN_NOSTART),
- [TRANS_STATE_COMPLETED] = (__TRANS_START |
- __TRANS_ATTACH |
- __TRANS_JOIN |
- __TRANS_JOIN_NOLOCK |
- __TRANS_JOIN_NOSTART),
+ [TRANS_STATE_COMMIT_START] = (TRANS_START | TRANS_ATTACH),
+ [TRANS_STATE_COMMIT_DOING] = (TRANS_START |
+ TRANS_ATTACH |
+ TRANS_JOIN |
+ TRANS_JOIN_NOSTART),
+ [TRANS_STATE_UNBLOCKED] = (TRANS_START |
+ TRANS_ATTACH |
+ TRANS_JOIN |
+ TRANS_JOIN_NOLOCK |
+ TRANS_JOIN_NOSTART),
+ [TRANS_STATE_SUPER_COMMITTED] = (TRANS_START |
+ TRANS_ATTACH |
+ TRANS_JOIN |
+ TRANS_JOIN_NOLOCK |
+ TRANS_JOIN_NOSTART),
+ [TRANS_STATE_COMPLETED] = (TRANS_START |
+ TRANS_ATTACH |
+ TRANS_JOIN |
+ TRANS_JOIN_NOLOCK |
+ TRANS_JOIN_NOSTART),
};
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
{
- return trans->type == __TRANS_DUMMY;
+ return trans->type == TRANS_DUMMY;
}
void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
@@ -172,7 +166,7 @@ void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
{
memset(trans, 0, sizeof(*trans));
trans->transid = 1;
- trans->type = __TRANS_DUMMY;
+ trans->type = TRANS_DUMMY;
trans->fs_info = fs_info;
}
#endif
@@ -261,21 +255,21 @@ static noinline void switch_commit_roots(struct btrfs_trans_handle *trans)
static inline void extwriter_counter_inc(struct btrfs_transaction *trans,
unsigned int type)
{
- if (type & TRANS_EXTWRITERS)
+ if (type & TRANS_EXTWRITERS_MASK)
atomic_inc(&trans->num_extwriters);
}
static inline void extwriter_counter_dec(struct btrfs_transaction *trans,
unsigned int type)
{
- if (type & TRANS_EXTWRITERS)
+ if (type & TRANS_EXTWRITERS_MASK)
atomic_dec(&trans->num_extwriters);
}
static inline void extwriter_counter_init(struct btrfs_transaction *trans,
unsigned int type)
{
- atomic_set(&trans->num_extwriters, ((type & TRANS_EXTWRITERS) ? 1 : 0));
+ atomic_set(&trans->num_extwriters, ((type & TRANS_EXTWRITERS_MASK) ? 1 : 0));
}
static inline int extwriter_counter_read(struct btrfs_transaction *trans)
@@ -662,11 +656,14 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
bool do_chunk_alloc = false;
int ret;
+ /* The type must be a single TRANS_* bit set. */
+ ASSERT(is_power_of_2(type));
+
if (unlikely(BTRFS_FS_ERROR(fs_info)))
return ERR_PTR(-EROFS);
if (current->journal_info) {
- WARN_ON(type & TRANS_EXTWRITERS);
+ WARN_ON(type & TRANS_EXTWRITERS_MASK);
h = current->journal_info;
refcount_inc(&h->use_count);
WARN_ON(refcount_read(&h->use_count) > 2);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/3] btrfs: remove __TRANS_* flags
2026-09-24 5:58 ` [PATCH v2 3/3] btrfs: remove __TRANS_* flags Qu Wenruo
@ 2026-09-28 18:34 ` Filipe Manana
0 siblings, 0 replies; 8+ messages in thread
From: Filipe Manana @ 2026-09-28 18:34 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Sep 24, 2026 at 7:01 AM Qu Wenruo <wqu@suse.com> wrote:
>
> After patch "btrfs: remove __TRANS_FREEZABLE", each TRANS_* flag is just
> the corresponding single-bit __TRANS_* flag.
>
> There is no need to split __TRANS_* and TRANS_* flags, just remove
> __TRANS_* flags and use TRANS_* flags instead.
>
> Since every TRANS_* flag is a single bit, do extra cleanups:
>
> - Add an ASSERT() in start_transaction()
> To make sure there is only a single bit set in @type
>
> - Rename TRANS_EXTWRITERS to TRANS_EXTWRITERS_MASK
> To follow the naming scheme that a multi-bit value has the _MASK
> suffix.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/transaction.c | 77 ++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 40 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index d73c8723ed70..2532b7b27f9a 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -39,26 +39,20 @@
> static struct kmem_cache *btrfs_trans_handle_cachep;
>
> enum {
> - ENUM_BIT(__TRANS_START),
> - ENUM_BIT(__TRANS_ATTACH),
> - ENUM_BIT(__TRANS_JOIN),
> - ENUM_BIT(__TRANS_JOIN_NOLOCK),
> + ENUM_BIT(TRANS_START),
> + ENUM_BIT(TRANS_ATTACH),
> + ENUM_BIT(TRANS_JOIN),
> + ENUM_BIT(TRANS_JOIN_NOLOCK),
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> - ENUM_BIT(__TRANS_DUMMY),
> + ENUM_BIT(TRANS_DUMMY),
> #endif
> - ENUM_BIT(__TRANS_JOIN_NOSTART),
> + ENUM_BIT(TRANS_JOIN_NOSTART),
> };
>
> -#define TRANS_START (__TRANS_START)
> -#define TRANS_ATTACH (__TRANS_ATTACH)
> -#define TRANS_JOIN (__TRANS_JOIN)
> -#define TRANS_JOIN_NOLOCK (__TRANS_JOIN_NOLOCK)
> -#define TRANS_JOIN_NOSTART (__TRANS_JOIN_NOSTART)
Ok, so this does what I asked for in the review of the previous patch,
so forget the previous comment.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> -
> /* Those types need to hold sb intwrite lock. */
> -#define TRANS_SB_INTWRITER_MASK (__TRANS_START | __TRANS_JOIN)
> +#define TRANS_SB_INTWRITER_MASK (TRANS_START | TRANS_JOIN)
>
> -#define TRANS_EXTWRITERS (__TRANS_START | __TRANS_ATTACH)
> +#define TRANS_EXTWRITERS_MASK (TRANS_START | TRANS_ATTACH)
>
> /*
> * Transaction states and transitions
> @@ -139,32 +133,32 @@ enum {
> static const unsigned int btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
> [TRANS_STATE_RUNNING] = 0U,
> [TRANS_STATE_COMMIT_PREP] = 0U,
> - [TRANS_STATE_COMMIT_START] = (__TRANS_START | __TRANS_ATTACH),
> - [TRANS_STATE_COMMIT_DOING] = (__TRANS_START |
> - __TRANS_ATTACH |
> - __TRANS_JOIN |
> - __TRANS_JOIN_NOSTART),
> - [TRANS_STATE_UNBLOCKED] = (__TRANS_START |
> - __TRANS_ATTACH |
> - __TRANS_JOIN |
> - __TRANS_JOIN_NOLOCK |
> - __TRANS_JOIN_NOSTART),
> - [TRANS_STATE_SUPER_COMMITTED] = (__TRANS_START |
> - __TRANS_ATTACH |
> - __TRANS_JOIN |
> - __TRANS_JOIN_NOLOCK |
> - __TRANS_JOIN_NOSTART),
> - [TRANS_STATE_COMPLETED] = (__TRANS_START |
> - __TRANS_ATTACH |
> - __TRANS_JOIN |
> - __TRANS_JOIN_NOLOCK |
> - __TRANS_JOIN_NOSTART),
> + [TRANS_STATE_COMMIT_START] = (TRANS_START | TRANS_ATTACH),
> + [TRANS_STATE_COMMIT_DOING] = (TRANS_START |
> + TRANS_ATTACH |
> + TRANS_JOIN |
> + TRANS_JOIN_NOSTART),
> + [TRANS_STATE_UNBLOCKED] = (TRANS_START |
> + TRANS_ATTACH |
> + TRANS_JOIN |
> + TRANS_JOIN_NOLOCK |
> + TRANS_JOIN_NOSTART),
> + [TRANS_STATE_SUPER_COMMITTED] = (TRANS_START |
> + TRANS_ATTACH |
> + TRANS_JOIN |
> + TRANS_JOIN_NOLOCK |
> + TRANS_JOIN_NOSTART),
> + [TRANS_STATE_COMPLETED] = (TRANS_START |
> + TRANS_ATTACH |
> + TRANS_JOIN |
> + TRANS_JOIN_NOLOCK |
> + TRANS_JOIN_NOSTART),
> };
>
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> bool btrfs_trans_is_dummy(const struct btrfs_trans_handle *trans)
> {
> - return trans->type == __TRANS_DUMMY;
> + return trans->type == TRANS_DUMMY;
> }
>
> void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> @@ -172,7 +166,7 @@ void btrfs_init_dummy_trans(struct btrfs_trans_handle *trans,
> {
> memset(trans, 0, sizeof(*trans));
> trans->transid = 1;
> - trans->type = __TRANS_DUMMY;
> + trans->type = TRANS_DUMMY;
> trans->fs_info = fs_info;
> }
> #endif
> @@ -261,21 +255,21 @@ static noinline void switch_commit_roots(struct btrfs_trans_handle *trans)
> static inline void extwriter_counter_inc(struct btrfs_transaction *trans,
> unsigned int type)
> {
> - if (type & TRANS_EXTWRITERS)
> + if (type & TRANS_EXTWRITERS_MASK)
> atomic_inc(&trans->num_extwriters);
> }
>
> static inline void extwriter_counter_dec(struct btrfs_transaction *trans,
> unsigned int type)
> {
> - if (type & TRANS_EXTWRITERS)
> + if (type & TRANS_EXTWRITERS_MASK)
> atomic_dec(&trans->num_extwriters);
> }
>
> static inline void extwriter_counter_init(struct btrfs_transaction *trans,
> unsigned int type)
> {
> - atomic_set(&trans->num_extwriters, ((type & TRANS_EXTWRITERS) ? 1 : 0));
> + atomic_set(&trans->num_extwriters, ((type & TRANS_EXTWRITERS_MASK) ? 1 : 0));
> }
>
> static inline int extwriter_counter_read(struct btrfs_transaction *trans)
> @@ -662,11 +656,14 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
> bool do_chunk_alloc = false;
> int ret;
>
> + /* The type must be a single TRANS_* bit set. */
> + ASSERT(is_power_of_2(type));
> +
> if (unlikely(BTRFS_FS_ERROR(fs_info)))
> return ERR_PTR(-EROFS);
>
> if (current->journal_info) {
> - WARN_ON(type & TRANS_EXTWRITERS);
> + WARN_ON(type & TRANS_EXTWRITERS_MASK);
> h = current->journal_info;
> refcount_inc(&h->use_count);
> WARN_ON(refcount_read(&h->use_count) > 2);
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread