Re: [PATCH bpf-next 1/2] btf: resolve enum fwds in btf_dedup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/10/19 5:44 PM, Andrii Nakryiko wrote:
> GCC and clang support enum forward declarations as an extension. Such
> forward-declared enums will be represented as normal BTF_KIND_ENUM types with
> vlen=0. This patch adds ability to resolve such enums to their corresponding

Thanks Andrii for reporting and trying to fix forward-declared enum 
issue. The current approach to represent forward enum with
BTF_KIND_ENUM with vlen = 0 does work. "enum A {};" is illegal for C,
so vlen=0 must be a forward declaration.

But this is probably not the best since we have BTF_KIND_FWD to make 
forward declaration explicit.

Currently BTF_KIND_FWD uses (btf_type.info >> 31) to distinguish
struct/union. If btf_type.info >> 31 == 0, it is a struct, otherwise
it is a union.

We could extend this to cover forward enum as well with backward 
compability. We can extend by using both bit 30 and bit 31:
   btf_type.info >> 31   (btf_type.info >> 30) & 0x1
         0                  0                  <== struct
         1                  0                  <== union
         0                  1                  <== enum

Maybe this may be reasonable?

This will require pahole and llvm change to support, besides kernel 
support. I will try to prototype llvm support tomorrow.


> fully defined enums. This helps to avoid duplicated BTF type graphs which only
> differ by some types referencing forward-declared enum vs full enum.
> 
> One such example in kernel is enum irqchip_irq_state, defined in
> include/linux/interrupt.h and forward-declared in include/linux/irq.h. This
> causes entire struct task_struct and all referenced types to be duplicated in
> btf_dedup output. This patch eliminates such duplication cases.
> 
> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> ---
>   tools/lib/bpf/btf.c | 51 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 1b8d8cdd3575..87e3020ac1bc 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1602,16 +1602,12 @@ static bool btf_equal_int(struct btf_type *t1, struct btf_type *t2)
>   /* Calculate type signature hash of ENUM. */
>   static __u32 btf_hash_enum(struct btf_type *t)
>   {
> -	struct btf_enum *member = (struct btf_enum *)(t + 1);
> -	__u32 vlen = BTF_INFO_VLEN(t->info);
> -	__u32 h = btf_hash_common(t);
> -	int i;
> +	__u32 h;
>   
> -	for (i = 0; i < vlen; i++) {
> -		h = hash_combine(h, member->name_off);
> -		h = hash_combine(h, member->val);
> -		member++;
> -	}
> +	/* don't hash vlen and enum members to support enum fwd resolving */
> +	h = hash_combine(0, t->name_off);
> +	h = hash_combine(h, t->info & ~0xffff);
> +	h = hash_combine(h, t->size);
>   	return h;
>   }
>   
> @@ -1637,6 +1633,22 @@ static bool btf_equal_enum(struct btf_type *t1, struct btf_type *t2)
>   	return true;
>   }
>   
> +static inline bool btf_is_enum_fwd(struct btf_type *t)
> +{
> +	return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM &&
> +	       BTF_INFO_VLEN(t->info) == 0;
> +}
> +
> +static bool btf_compat_enum(struct btf_type *t1, struct btf_type *t2)
> +{
> +	if (!btf_is_enum_fwd(t1) && !btf_is_enum_fwd(t2))
> +		return btf_equal_enum(t1, t2);
> +	/* ignore vlen when comparing */
> +	return t1->name_off == t2->name_off &&
> +	       (t1->info & ~0xffff) == (t2->info & ~0xffff) &&
> +	       t1->size == t2->size;
> +}
> +
>   /*
>    * Calculate type signature hash of STRUCT/UNION, ignoring referenced type IDs,
>    * as referenced type IDs equivalence is established separately during type
> @@ -1860,6 +1872,17 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
>   				new_id = cand_node->type_id;
>   				break;
>   			}
> +			if (d->opts.dont_resolve_fwds)
> +				continue;
> +			if (btf_compat_enum(t, cand)) {
> +				if (btf_is_enum_fwd(t)) {
> +					/* resolve fwd to full enum */
> +					new_id = cand_node->type_id;
> +					break;
> +				}
> +				/* resolve canonical enum fwd to full enum */
> +				d->map[cand_node->type_id] = type_id;
> +			}
>   		}
>   		break;
>   
> @@ -2084,15 +2107,15 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
>   		return fwd_kind == real_kind;
>   	}
>   
> -	if (cand_type->info != canon_type->info)
> -		return 0;
> -
>   	switch (cand_kind) {
>   	case BTF_KIND_INT:
>   		return btf_equal_int(cand_type, canon_type);
>   
>   	case BTF_KIND_ENUM:
> -		return btf_equal_enum(cand_type, canon_type);
> +		if (d->opts.dont_resolve_fwds)
> +			return btf_equal_enum(cand_type, canon_type);
> +		else
> +			return btf_compat_enum(cand_type, canon_type);
>   
>   	case BTF_KIND_FWD:
>   		return btf_equal_common(cand_type, canon_type);
> @@ -2103,6 +2126,8 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
>   	case BTF_KIND_PTR:
>   	case BTF_KIND_TYPEDEF:
>   	case BTF_KIND_FUNC:
> +		if (cand_type->info != canon_type->info)
> +			return 0;
>   		return btf_dedup_is_equiv(d, cand_type->type, canon_type->type);
>   
>   	case BTF_KIND_ARRAY: {
> 




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux