* [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction()
@ 2026-04-28 15:51 David Sterba
2026-04-28 22:45 ` Qu Wenruo
2026-04-29 14:32 ` [PATCH v2] " David Sterba
0 siblings, 2 replies; 6+ messages in thread
From: David Sterba @ 2026-04-28 15:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The btrfs_abort_transaction() is called at the location where we want to
report the abort. It must be a macro so we get the correct line and
stack trace. This inlines the necessary code and the rest is pushed to
__btrfs_abort_transaction().
There's a possibility to reduce the inlined code if we move the message
to the helper function as well, without loss of information. The
difference is only that the WARN will not print it inside the stack
report but after:
--[ cut here ]--
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
BTRFS error (device dm-0 state A): Transaction aborted (error -28)
While previously there would be one more line like:
--[ cut here ]--
BTRFS: Transaction aborted (error -28)
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
This removes about 22KiB of btrfs.ko on a release config.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/transaction.c | 3 +++
fs/btrfs/transaction.h | 11 +----------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 194f581b36f3..5125cb20ef85 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2729,6 +2729,9 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
{
struct btrfs_fs_info *fs_info = trans->fs_info;
+ btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
+ trans->transid, error);
+
WRITE_ONCE(trans->aborted, error);
WRITE_ONCE(trans->transaction->aborted, error);
if (first_hit && error == -ENOSPC)
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 264dcd4b3788..f1cb05460cec 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -253,16 +253,7 @@ do { \
if (!test_and_set_bit(BTRFS_FS_STATE_TRANS_ABORTED, \
&((trans)->fs_info->fs_state))) { \
__first = true; \
- if (WARN(btrfs_abort_should_print_stack(error), \
- KERN_ERR \
- "BTRFS: Transaction %llu aborted (error %d)\n", \
- (trans)->transid, (error))) { \
- /* Stack trace printed. */ \
- } else { \
- btrfs_err((trans)->fs_info, \
- "Transaction %llu aborted (error %d)", \
- (trans)->transid, (error)); \
- } \
+ WARN_ON(btrfs_abort_should_print_stack(error)); \
} \
__btrfs_abort_transaction((trans), __func__, \
__LINE__, (error), __first); \
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction()
2026-04-28 15:51 [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction() David Sterba
@ 2026-04-28 22:45 ` Qu Wenruo
2026-04-29 14:19 ` David Sterba
2026-04-29 14:32 ` [PATCH v2] " David Sterba
1 sibling, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2026-04-28 22:45 UTC (permalink / raw)
To: David Sterba, linux-btrfs
在 2026/4/29 01:21, David Sterba 写道:
> The btrfs_abort_transaction() is called at the location where we want to
> report the abort. It must be a macro so we get the correct line and
> stack trace. This inlines the necessary code and the rest is pushed to
> __btrfs_abort_transaction().
>
> There's a possibility to reduce the inlined code if we move the message
> to the helper function as well, without loss of information. The
> difference is only that the WARN will not print it inside the stack
> report but after:
>
> --[ cut here ]--
> WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> ...
> --[ end trace ] --
> BTRFS error (device dm-0 state A): Transaction aborted (error -28)
>
> While previously there would be one more line like:
>
> --[ cut here ]--
> BTRFS: Transaction aborted (error -28)
> WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> ...
> --[ end trace ] --
>
> This removes about 22KiB of btrfs.ko on a release config.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/transaction.c | 3 +++
> fs/btrfs/transaction.h | 11 +----------
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 194f581b36f3..5125cb20ef85 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2729,6 +2729,9 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
> {
> struct btrfs_fs_info *fs_info = trans->fs_info;
>
> + btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
> + trans->transid, error);
> +
This message will be outputted for every btrfs_abort_transaction() call.
Shouldn't we move it behind first_hit check?
Thanks,
Qu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction()
2026-04-28 22:45 ` Qu Wenruo
@ 2026-04-29 14:19 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2026-04-29 14:19 UTC (permalink / raw)
To: Qu Wenruo; +Cc: David Sterba, linux-btrfs
On Wed, Apr 29, 2026 at 08:15:39AM +0930, Qu Wenruo wrote:
>
>
> 在 2026/4/29 01:21, David Sterba 写道:
> > The btrfs_abort_transaction() is called at the location where we want to
> > report the abort. It must be a macro so we get the correct line and
> > stack trace. This inlines the necessary code and the rest is pushed to
> > __btrfs_abort_transaction().
> >
> > There's a possibility to reduce the inlined code if we move the message
> > to the helper function as well, without loss of information. The
> > difference is only that the WARN will not print it inside the stack
> > report but after:
> >
> > --[ cut here ]--
> > WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> > ...
> > --[ end trace ] --
> > BTRFS error (device dm-0 state A): Transaction aborted (error -28)
> >
> > While previously there would be one more line like:
> >
> > --[ cut here ]--
> > BTRFS: Transaction aborted (error -28)
> > WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> > ...
> > --[ end trace ] --
> >
> > This removes about 22KiB of btrfs.ko on a release config.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/transaction.c | 3 +++
> > fs/btrfs/transaction.h | 11 +----------
> > 2 files changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> > index 194f581b36f3..5125cb20ef85 100644
> > --- a/fs/btrfs/transaction.c
> > +++ b/fs/btrfs/transaction.c
> > @@ -2729,6 +2729,9 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
> > {
> > struct btrfs_fs_info *fs_info = trans->fs_info;
> >
> > + btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
> > + trans->transid, error);
> > +
>
> This message will be outputted for every btrfs_abort_transaction() call.
>
> Shouldn't we move it behind first_hit check?
Right, we should, to keep the previous behaviour. I'll update it,
thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] btrfs: move transaction abort message to __btrfs_abort_transaction()
2026-04-28 15:51 [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction() David Sterba
2026-04-28 22:45 ` Qu Wenruo
@ 2026-04-29 14:32 ` David Sterba
2026-04-29 14:35 ` Filipe Manana
1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2026-04-29 14:32 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The btrfs_abort_transaction() is called at the location where we want to
report the abort. It must be a macro so we get the correct line and
stack trace. This inlines the necessary code and the rest is pushed to
__btrfs_abort_transaction().
There's a possibility to reduce the inlined code if we move the message
to the helper function as well, without loss of information. The
difference is only that the WARN will not print it inside the stack
report but after:
--[ cut here ]--
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
BTRFS error (device dm-0 state A): Transaction aborted (error -28)
While previously there would be one more line like:
--[ cut here ]--
BTRFS: Transaction aborted (error -28)
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
This removes about 20KiB of btrfs.ko on a release config.
Signed-off-by: David Sterba <dsterba@suse.com>
---
v2:
- move message in __btrfs_abort_transaction() under 'first_hit', also to
make the WRITE_ONCE() assignments immediate
- savings updated to 20K, it depends on the config and compiler, use an
average of a few setups
fs/btrfs/transaction.c | 8 ++++++--
fs/btrfs/transaction.h | 11 +----------
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 194f581b36f3..184fe976450e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2731,8 +2731,12 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
WRITE_ONCE(trans->aborted, error);
WRITE_ONCE(trans->transaction->aborted, error);
- if (first_hit && error == -ENOSPC)
- btrfs_dump_space_info_for_trans_abort(fs_info);
+ if (first_hit) {
+ btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
+ trans->transid, error);
+ if (error == -ENOSPC)
+ btrfs_dump_space_info_for_trans_abort(fs_info);
+ }
/* Wake up anybody who may be waiting on this transaction */
wake_up(&fs_info->transaction_wait);
wake_up(&fs_info->transaction_blocked_wait);
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 7d70fe486758..f1cb05460cec 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -253,16 +253,7 @@ do { \
if (!test_and_set_bit(BTRFS_FS_STATE_TRANS_ABORTED, \
&((trans)->fs_info->fs_state))) { \
__first = true; \
- if (WARN(btrfs_abort_should_print_stack(error), \
- KERN_ERR \
- "BTRFS: Transaction aborted (error %d)\n", \
- (error))) { \
- /* Stack trace printed. */ \
- } else { \
- btrfs_err((trans)->fs_info, \
- "Transaction aborted (error %d)", \
- (error)); \
- } \
+ WARN_ON(btrfs_abort_should_print_stack(error)); \
} \
__btrfs_abort_transaction((trans), __func__, \
__LINE__, (error), __first); \
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: move transaction abort message to __btrfs_abort_transaction()
2026-04-29 14:32 ` [PATCH v2] " David Sterba
@ 2026-04-29 14:35 ` Filipe Manana
2026-04-29 14:59 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Filipe Manana @ 2026-04-29 14:35 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Wed, Apr 29, 2026 at 3:33 PM David Sterba <dsterba@suse.com> wrote:
>
> The btrfs_abort_transaction() is called at the location where we want to
> report the abort. It must be a macro so we get the correct line and
> stack trace. This inlines the necessary code and the rest is pushed to
> __btrfs_abort_transaction().
>
> There's a possibility to reduce the inlined code if we move the message
> to the helper function as well, without loss of information. The
> difference is only that the WARN will not print it inside the stack
> report but after:
>
> --[ cut here ]--
> WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> ...
> --[ end trace ] --
> BTRFS error (device dm-0 state A): Transaction aborted (error -28)
>
> While previously there would be one more line like:
>
> --[ cut here ]--
> BTRFS: Transaction aborted (error -28)
> WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> ...
> --[ end trace ] --
>
> This removes about 20KiB of btrfs.ko on a release config.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>
> v2:
> - move message in __btrfs_abort_transaction() under 'first_hit', also to
> make the WRITE_ONCE() assignments immediate
> - savings updated to 20K, it depends on the config and compiler, use an
> average of a few setups
>
> fs/btrfs/transaction.c | 8 ++++++--
> fs/btrfs/transaction.h | 11 +----------
> 2 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 194f581b36f3..184fe976450e 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2731,8 +2731,12 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
>
> WRITE_ONCE(trans->aborted, error);
> WRITE_ONCE(trans->transaction->aborted, error);
> - if (first_hit && error == -ENOSPC)
> - btrfs_dump_space_info_for_trans_abort(fs_info);
> + if (first_hit) {
> + btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
There's already a local fs_info variable, no need to trans->fs_info.
Otherwise:
Filipe Manana <fdmanana@suse.com>
> + trans->transid, error);
> + if (error == -ENOSPC)
> + btrfs_dump_space_info_for_trans_abort(fs_info);
> + }
> /* Wake up anybody who may be waiting on this transaction */
> wake_up(&fs_info->transaction_wait);
> wake_up(&fs_info->transaction_blocked_wait);
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 7d70fe486758..f1cb05460cec 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -253,16 +253,7 @@ do { \
> if (!test_and_set_bit(BTRFS_FS_STATE_TRANS_ABORTED, \
> &((trans)->fs_info->fs_state))) { \
> __first = true; \
> - if (WARN(btrfs_abort_should_print_stack(error), \
> - KERN_ERR \
> - "BTRFS: Transaction aborted (error %d)\n", \
> - (error))) { \
> - /* Stack trace printed. */ \
> - } else { \
> - btrfs_err((trans)->fs_info, \
> - "Transaction aborted (error %d)", \
> - (error)); \
> - } \
> + WARN_ON(btrfs_abort_should_print_stack(error)); \
> } \
> __btrfs_abort_transaction((trans), __func__, \
> __LINE__, (error), __first); \
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: move transaction abort message to __btrfs_abort_transaction()
2026-04-29 14:35 ` Filipe Manana
@ 2026-04-29 14:59 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2026-04-29 14:59 UTC (permalink / raw)
To: Filipe Manana; +Cc: David Sterba, linux-btrfs
On Wed, Apr 29, 2026 at 03:35:55PM +0100, Filipe Manana wrote:
> On Wed, Apr 29, 2026 at 3:33 PM David Sterba <dsterba@suse.com> wrote:
> >
> > The btrfs_abort_transaction() is called at the location where we want to
> > report the abort. It must be a macro so we get the correct line and
> > stack trace. This inlines the necessary code and the rest is pushed to
> > __btrfs_abort_transaction().
> >
> > There's a possibility to reduce the inlined code if we move the message
> > to the helper function as well, without loss of information. The
> > difference is only that the WARN will not print it inside the stack
> > report but after:
> >
> > --[ cut here ]--
> > WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> > ...
> > --[ end trace ] --
> > BTRFS error (device dm-0 state A): Transaction aborted (error -28)
> >
> > While previously there would be one more line like:
> >
> > --[ cut here ]--
> > BTRFS: Transaction aborted (error -28)
> > WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
> > ...
> > --[ end trace ] --
> >
> > This removes about 20KiB of btrfs.ko on a release config.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >
> > v2:
> > - move message in __btrfs_abort_transaction() under 'first_hit', also to
> > make the WRITE_ONCE() assignments immediate
> > - savings updated to 20K, it depends on the config and compiler, use an
> > average of a few setups
> >
> > fs/btrfs/transaction.c | 8 ++++++--
> > fs/btrfs/transaction.h | 11 +----------
> > 2 files changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> > index 194f581b36f3..184fe976450e 100644
> > --- a/fs/btrfs/transaction.c
> > +++ b/fs/btrfs/transaction.c
> > @@ -2731,8 +2731,12 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
> >
> > WRITE_ONCE(trans->aborted, error);
> > WRITE_ONCE(trans->transaction->aborted, error);
> > - if (first_hit && error == -ENOSPC)
> > - btrfs_dump_space_info_for_trans_abort(fs_info);
> > + if (first_hit) {
> > + btrfs_err(trans->fs_info, "Transaction %llu aborted (error %d)",
>
> There's already a local fs_info variable, no need to trans->fs_info.
Ah, copy&paste leftover, I'll fix it in for-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-29 14:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 15:51 [PATCH] btrfs: move transaction abort message to __btrfs_abort_transaction() David Sterba
2026-04-28 22:45 ` Qu Wenruo
2026-04-29 14:19 ` David Sterba
2026-04-29 14:32 ` [PATCH v2] " David Sterba
2026-04-29 14:35 ` Filipe Manana
2026-04-29 14:59 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox