On Wed, Oct 16, 2024 at 11:26:30PM -0400, Kent Overstreet wrote: > yeah, I think we would prefer smaller codesize, by default. > > it'd be well worth checking the code size difference on inlined vs. not, > and then the really think to do would be to provide optional _inlined() > helpers that we can switch to if/when a particular codepath shows up in > a profile Make sure to build with kCFI and IBT enabled when you do this and enjoy seeing ec_stripes_heap_cmp turn into this: 00000000000027c0 <__cfi_ec_stripes_heap_cmp>: 27c0: b8 01 88 88 07 mov $0x7888801,%eax 27c5: 90 nop 27c6: 90 nop 27c7: 90 nop 27c8: 90 nop 27c9: 90 nop 27ca: 90 nop 27cb: 90 nop 27cc: 90 nop 27cd: 90 nop 27ce: 90 nop 27cf: 90 nop 00000000000027d0 <ec_stripes_heap_cmp>: 27d0: f3 0f 1e fa endbr64 27d4: e8 00 00 00 00 call 27d9 <ec_stripes_heap_cmp+0x9> 27d5: R_X86_64_PLT32 __fentry__-0x4 27d9: 8b 47 08 mov 0x8(%rdi),%eax 27dc: 3b 46 08 cmp 0x8(%rsi),%eax 27df: 0f 92 c0 setb %al 27e2: 2e e9 00 00 00 00 cs jmp 27e8 <ec_stripes_heap_cmp+0x18> 27e4: R_X86_64_PLT32 __x86_return_thunk-0x4 27e8: 0f 1f 84 00 00 00 00 00 nopl 0x0(%rax,%rax,1) 27f0: 90 nop 27f1: 90 nop 27f2: 90 nop 27f3: 90 nop 27f4: 90 nop While I was there, you might want to do the below. It makes no sense duplicating that thing. --- diff --git a/fs/bcachefs/ec.c b/fs/bcachefs/ec.c index 1587c6e1866a..3a6c34cf8a15 100644 --- a/fs/bcachefs/ec.c +++ b/fs/bcachefs/ec.c @@ -1048,6 +1048,11 @@ static inline void ec_stripes_heap_swap(void *l, void *r, void *h) ec_stripes_heap_set_backpointer(_h, j); } +static const struct min_heap_callbacks ec_stripes_callbacks = { + .less = ec_stripes_heap_cmp, + .swp = ec_stripes_heap_swap, +}; + static void heap_verify_backpointer(struct bch_fs *c, size_t idx) { ec_stripes_heap *h = &c->ec_stripes_heap; @@ -1060,26 +1065,16 @@ static void heap_verify_backpointer(struct bch_fs *c, size_t idx) void bch2_stripes_heap_del(struct bch_fs *c, struct stripe *m, size_t idx) { - const struct min_heap_callbacks callbacks = { - .less = ec_stripes_heap_cmp, - .swp = ec_stripes_heap_swap, - }; - mutex_lock(&c->ec_stripes_heap_lock); heap_verify_backpointer(c, idx); - min_heap_del(&c->ec_stripes_heap, m->heap_idx, &callbacks, &c->ec_stripes_heap); + min_heap_del(&c->ec_stripes_heap, m->heap_idx, &ec_strips_callbacks, &c->ec_stripes_heap); mutex_unlock(&c->ec_stripes_heap_lock); } void bch2_stripes_heap_insert(struct bch_fs *c, struct stripe *m, size_t idx) { - const struct min_heap_callbacks callbacks = { - .less = ec_stripes_heap_cmp, - .swp = ec_stripes_heap_swap, - }; - mutex_lock(&c->ec_stripes_heap_lock); BUG_ON(min_heap_full(&c->ec_stripes_heap)); @@ -1088,7 +1083,7 @@ void bch2_stripes_heap_insert(struct bch_fs *c, .idx = idx, .blocks_nonempty = m->blocks_nonempty, }), - &callbacks, + &ec_stripes_callbacks, &c->ec_stripes_heap); heap_verify_backpointer(c, idx); @@ -1098,10 +1093,6 @@ void bch2_stripes_heap_insert(struct bch_fs *c, void bch2_stripes_heap_update(struct bch_fs *c, struct stripe *m, size_t idx) { - const struct min_heap_callbacks callbacks = { - .less = ec_stripes_heap_cmp, - .swp = ec_stripes_heap_swap, - }; ec_stripes_heap *h = &c->ec_stripes_heap; bool do_deletes; size_t i; @@ -1112,8 +1103,8 @@ void bch2_stripes_heap_update(struct bch_fs *c, h->data[m->heap_idx].blocks_nonempty = m->blocks_nonempty; i = m->heap_idx; - min_heap_sift_up(h, i, &callbacks, &c->ec_stripes_heap); - min_heap_sift_down(h, i, &callbacks, &c->ec_stripes_heap); + min_heap_sift_up( h, i, &ec_stripes_callbacks, &c->ec_stripes_heap); + min_heap_sift_down(h, i, &ec_stripes_callbacks, &c->ec_stripes_heap); heap_verify_backpointer(c, idx);