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);
