| Message ID | 20201124144502.3168362-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Remove noinline attribute from wait_for_commit | expand |
On Tue, Nov 24, 2020 at 04:45:02PM +0200, Nikolay Borisov wrote:
> The function is a plain wrapper that noinline makes no sense.
Or it is a way to let the function name appear in a stack trace, which
could be useful for debugging or analyzing system state.
On 24.11.20 г. 17:39 ч., David Sterba wrote: > On Tue, Nov 24, 2020 at 04:45:02PM +0200, Nikolay Borisov wrote: >> The function is a plain wrapper that noinline makes no sense. > > Or it is a way to let the function name appear in a stack trace, which > could be useful for debugging or analyzing system state. > Well, this information could generally be derived by having the debug info either in crash or one of the "beautify" scripts. In any case I won't insist.
On Tue, Nov 24, 2020 at 05:42:57PM +0200, Nikolay Borisov wrote: > > > On 24.11.20 г. 17:39 ч., David Sterba wrote: > > On Tue, Nov 24, 2020 at 04:45:02PM +0200, Nikolay Borisov wrote: > >> The function is a plain wrapper that noinline makes no sense. > > > > Or it is a way to let the function name appear in a stack trace, which > > could be useful for debugging or analyzing system state. > > > > Well, this information could generally be derived by having the debug > info either in crash or one of the "beautify" scripts. In any case I > won't insist. The way it's used is 'cat /proc/*/stack', without the need of debugging kernels and not in a post-mortem analysis with crash.
On Tue, Nov 24, 2020 at 05:05:32PM +0100, David Sterba wrote: > On Tue, Nov 24, 2020 at 05:42:57PM +0200, Nikolay Borisov wrote: > > On 24.11.20 г. 17:39 ч., David Sterba wrote: > > > On Tue, Nov 24, 2020 at 04:45:02PM +0200, Nikolay Borisov wrote: > > >> The function is a plain wrapper that noinline makes no sense. > > > > > > Or it is a way to let the function name appear in a stack trace, which > > > could be useful for debugging or analyzing system state. > > > > > > > Well, this information could generally be derived by having the debug > > info either in crash or one of the "beautify" scripts. In any case I > > won't insist. > > The way it's used is 'cat /proc/*/stack', without the need of debugging > kernels and not in a post-mortem analysis with crash. Actually, there's noinline_for_stack annotation that could make a bit more understandable, but reading the comment it's more about conserving stack space, not making the function visible in stack trace. There are functions like free_reloc_roots, memcmp_node_keys or cleanup_bitmap_list that do a trivial operations, no chance to block or wait. So these are the obvious cases where we don't want it, there are still many more for long functions that would need closer inspection.
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index e5a5c3604a9b..fd4293cf69cf 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -829,7 +829,7 @@ btrfs_attach_transaction_barrier(struct btrfs_root *root) } /* wait for a transaction commit to be fully complete */ -static noinline void wait_for_commit(struct btrfs_transaction *commit) +static void wait_for_commit(struct btrfs_transaction *commit) { wait_event(commit->commit_wait, commit->state == TRANS_STATE_COMPLETED); }
The function is a plain wrapper that noinline makes no sense. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.25.1