On 2024/8/23 22:51, Kent Overstreet wrote:
On Fri, Aug 23, 2024 at 02:07:20PM GMT, Youling Tang wrote:
On 23/08/2024 11:55, Kent Overstreet wrote:
On Fri, Aug 23, 2024 at 11:19:55AM GMT, Youling Tang wrote:
From: Youling Tang <tangyoul...@kylinos.cn>
- Reduces bkey_err() calls.
- Avoid redundant calls to bch2_trans_iter_exit() in some functions.
no, a function that returns an error should clean up after itself
Yes, functions should self-clean when they fail.
However, there are repeated calls to bch2_trans_iter_exit in
some functions, take lookup_inode() as an example,
When bkey_err(k) returns a non-zero, call bch2_trans_iter_exit()
once in bch2_bkey_get_iter(). It is then called again in
lookup_inode() via 'goto err'. (We can correct it by simply changing
it to 'return ret', but there are many similar cases.)
I'm aware, but I'm not looking to microoptimize at the expense of making
the code more fragile and less clear, especially right now when the
priority is stabilizing and fixing bugs.
If you were also doing performance testing and could show that it
makes a measurable difference I'd consider it. Did you even look at the
assembly output for any of these functions? CSE might be optimizing away
the redundant calls.
I haven't performed the corresponding performance testing. Looking at
the assembly code, taking `lookup_inode()` as an example,
Before the patch,
142f: 74 96 je 13c7 <lookup_inode+0x117>
1431: 48 8d b5 68 ff ff ff lea -0x98(%rbp),%rsi
1438: 4c 89 e7 mov %r12,%rdi
143b: e8 00 00 00 00 call 1440 <lookup_inode+0x190>
1440: eb b4 jmp 13f6 <lookup_inode+0x146>
1442: e8 00 00 00 00 call 1447 <lookup_inode+0x197>
1447: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
144e: 00 00
After,
111f: 74 96 je 10b7 <lookup_inode+0x117>
1121: eb c3 jmp 10e6 <lookup_inode+0x146>
1123: e8 00 00 00 00 call 1128 <lookup_inode+0x188>
1128: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
112f: 00
The following three assembly instructions have been reduced,
1431: 48 8d b5 68 ff ff ff lea -0x98(%rbp),%rsi
1438: 4c 89 e7 mov %r12,%rdi
143b: e8 00 00 00 00 call 1440 <lookup_inode+0x190>
