Re: [PATCH v2 nf 1/3] nft_set_pipapo: add avx register usage tracking for NET_DEBUG builds

[Florian Westphal <fw@strlen.de>]

Stefano Brivio <sbrivio@redhat.com> wrote:
> > +#ifdef CONFIG_DEBUG_NET
> > +/* YYM15 is used as an always-0-register, see nft_pipapo_avx2_prepare */
> 
> It's really YMM (or ymm), with two m's and one y. :) That's what I was
> referring to in my previous comment.

Grr, my brain autocorrects this for some reason :-/

I'll fix it in v3 on Monday.

> > +#define NFT_PIPAPO_AVX2_DEBUG_MAP                                       \
> > +	struct nft_pipapo_debug_regmap __pipapo_debug_regmap = {        \
> > +		.tmp = BIT(15),                                         \
> > +	}
> 
> This mixes spaces and tabs (I guess from copy and paste).

Thanks, will fix it too.

> > +#ifdef CONFIG_DEBUG_NET
> > +	bool holds_and_result = BIT(reg) & r->tmp;
> > +
> > +        NFT_PIPAPO_WARN(!holds_and_result, reg, r, line, "unused");
> 
> This is indented with spaces.

Indeed, will fix.

> > @@ -687,6 +871,7 @@ static int nft_pipapo_avx2_lookup_8b_1(unsigned long *map, unsigned long *fill,
> >  
> >  		if (first) {
> >  			NFT_PIPAPO_AVX2_BUCKET_LOAD8(2, lt, 0, pkt[0], bsize);
> > +			nft_pipapo_avx2_force_tmp(2, &__pipapo_debug_regmap);
> 
> Right, that's because we have an 8-bit bucket and we're comparing 8
> bits, so in this case we don't need to AND any value in the first
> iteration.

Thanks for confirming.