On Sun, 23 Feb 2020 23:14:35 +0100
Florian Westphal <fw@xxxxxxxxx> wrote:
> Stefano Brivio <sbrivio@xxxxxxxxxx> wrote:
> > struct nft_pipapo_field {
> > @@ -439,6 +456,9 @@ struct nft_pipapo_field {
> > unsigned long rules;
> > size_t bsize;
> > int bb;
> > +#ifdef NFT_PIPAPO_ALIGN
> > + unsigned long *lt_aligned;
> > +#endif
> > unsigned long *lt;
> > union nft_pipapo_map_bucket *mt;
> > };
>
> I wonder if these structs can be compressed.
> AFAICS bsize is in sizes of longs, so when this number is
> large then we also need to kvmalloc a large blob of memory.
>
> I think u32 would be enough?
Hm, yes, it is. I just thought it was a "good idea" to use size_t for
"sizes", but this is so implementation-specific that u32 would make
total sense (it's enough for 32GiB), and avoid holes on 64-bit archs.
> nft_pipapo_field is probably the most relevant one wrt. to size.
Right. I forgot about that when I added 'bb'.
> > struct nft_pipapo_match {
> > int field_count;
> > +#ifdef NFT_PIPAPO_ALIGN
> > + unsigned long * __percpu *scratch_aligned;
> > +#endif
> > unsigned long * __percpu *scratch;
> > size_t bsize_max;
>
> Same here (bsize_max -- could fit with hole after field_count)?
Yes, makes sense.
> Also, since you know the size of nft_pipapo_match (including the
> dynamically allocated array at the end), you could store the
> original memory (*scratch) and the rcu_head at the end, since
> they are not needed at lookup time and a little overhead to calculate
> their storage offset is fine.
>
> Not sure its worth it, just an idea.
'*scratch' is actually needed at lookup time for implementations that
don't need stricter alignment than natural one, but I could probably
use some macro trickery and "move" it as needed.
I'm not sure how to deal with fields after f[0], syntactically. Do you
have some, er, pointers?
--
Stefano
