On Tue, Nov 16, 2021 at 04:38:18PM -0800, Andrii Nakryiko wrote:
> >
> > -static inline u32 btf_member_bit_offset(const struct btf_type *struct_type,
> > - const struct btf_member *member)
> > +static inline u32 __btf_member_bit_offset(const struct btf_type *struct_type,
> > + const struct btf_member *member)
>
> a bit surprised you didn't choose to just remove them, given you had
> to touch all 24 places in the kernel that call this helper
> > - if (btf_member_bitfield_size(t, member)) {
> > + if (__btf_member_bitfield_size(t, member)) {
>
> like in this case it would be btf_member_bitfield_size(t, j)
In this and few other cases it's indeed possible, but not in net/ipv4/bpf_tcp_ca.c
It has two callbacks:
struct bpf_struct_ops {
const struct bpf_verifier_ops *verifier_ops;
int (*init)(struct btf *btf);
int (*check_member)(const struct btf_type *t,
const struct btf_member *member);
int (*init_member)(const struct btf_type *t,
const struct btf_member *member,
so they cannot be changed without massive refactoring.
Also member pointer vs index is arguably a better api.
I'm not sure compiler can optimize index into pointer in case like below
and won't introduce redundant loads.
> > for_each_member(i, t, member) {
> > - moff = btf_member_bit_offset(t, member) / 8;
> > + moff = __btf_member_bit_offset(t, member) / 8;
>
> same here, seema like in all the cases we already have member_idx (i
> in this case)
