Re: [PATCH nf] nft_set_pipapo: Disable preemption before getting per-CPU pointer

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 8 Jun 2020 10:56:52 +0200
Florian Westphal <fw@strlen.de> wrote:

> Stefano Brivio <sbrivio@redhat.com> wrote:
> > The lkp kernel test robot reports, with CONFIG_DEBUG_PREEMPT enabled:  
> [..]
> 
> > diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> > index 8b5acc6910fd..8c04388296b0 100644
> > --- a/net/netfilter/nft_set_pipapo.c
> > +++ b/net/netfilter/nft_set_pipapo.c
> > @@ -1242,7 +1242,9 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
> >  		end += NFT_PIPAPO_GROUPS_PADDED_SIZE(f);
> >  	}
> >  
> > -	if (!*this_cpu_ptr(m->scratch) || bsize_max > m->bsize_max) {
> > +	if (!*get_cpu_ptr(m->scratch) || bsize_max > m->bsize_max) {
> > +		put_cpu_ptr(m->scratch);
> > +
> >  		err = pipapo_realloc_scratch(m, bsize_max);
> >  		if (err)
> >  			return err;
> > @@ -1250,6 +1252,8 @@ static int nft_pipapo_insert(const struct net *net, const struct nft_set *set,
> >  		this_cpu_write(nft_pipapo_scratch_index, false);  
> 
> Won't that mean that this_cpu_write() can occur on a different CPU than
> the get_cpu_ptr() ptr check was done on?

Yes, but that's okay, because get_cpu_ptr(m->scratch) can be done on
any CPU: it's just used as a flag to detect that scratch maps were
never allocated for this matching data.

The write to nft_pipapo_scratch_index is not needed: in this path we
always call pipapo_realloc_scratch() so we're supplying two fresh
scratch maps, we can start from either one for the next lookup
operation. But dropping it doesn't fix anything, so I'd do that in a
separate patch for nf-next.

-- 
Stefano