RE: Fwd: Re: Kernel panic with 4.16-rc1 (and 4.16-rc2) running selftest

[Chris Mi]

> -----Original Message-----
> From: Matthew Wilcox [mailto:willy@xxxxxxxxxxxxx]
> Sent: Saturday, February 24, 2018 9:15 AM
> To: Cong Wang <xiyou.wangcong@xxxxxxxxx>; Khalid Aziz
> <khalid.aziz@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx
> Cc: Chris Mi <chrism@xxxxxxxxxxxx>
> Subject: Re: Fwd: Re: Kernel panic with 4.16-rc1 (and 4.16-rc2) running
> selftest
> 
> On Fri, Feb 23, 2018 Randy Dunlap wrote:
> > [add Matthew Wilcox; hopefully he can look/see]
> 
> Thanks, Randy.  I don't understand why nobody else thought to cc the author
> of the patch that it was bisected to ...
> 
> > On 02/23/2018 04:13 PM, Cong Wang wrote:
> > > On Fri, Feb 23, 2018 at 3:27 PM, Cong Wang
> > > <xiyou.wangcong@xxxxxxxxx>
> > wrote:
> > >> On Fri, Feb 23, 2018 at 11:00 AM, Randy Dunlap
> > >> <rdunlap@xxxxxxxxxxxxx>
> > wrote:
> > >>> On 02/23/2018 08:05 AM, Khalid Aziz wrote:
> > >>>> Same selftest does not cause panic on 4.15. git bisect pointed to
> > commit 6ce711f2750031d12cec91384ac5cfa0a485b60a ("idr: Make 1-based
> > IDRs more efficient").
> > >>>> Kernel config is attached.
> > >>
> > >> Looks like something horribly wrong with u32 key id idr...
> > >
> > > Adding a few printk's, I got:
> > >
> > > [   31.231560] requested handle = ffe00000
> > > [   31.232426] allocated handle = 0
> > > ...
> > > [   31.246475] requested handle = ffd00000
> > > [   31.247555] allocated handle = 1
> > >
> > >
> > > So the bug is here where we can't allocate a specific handle:
> > >
> > >                         err = idr_alloc_u32(&tp_c->handle_idr, ht,
> > &handle,
> > >                                             handle, GFP_KERNEL);
> > >                         if (err) {
> > >                                 kfree(ht);
> > >                                 return err;
> > >                         }
> 
> Please try this patch.  It fixes ffe00000, but there may be more things tested
> that it may not work for.
> 
> Chris Mi, what happened to that set of testcases you promised to write for
> me?
I promised to write it after the API is stabilized since you were going to change it.
I will inform the management about this new task and get back to you later.
> 
> diff --git a/lib/idr.c b/lib/idr.c
> index c98d77fcf393..10d9b8d47c33 100644
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -36,8 +36,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid,  {
>  	struct radix_tree_iter iter;
>  	void __rcu **slot;
> -	int base = idr->idr_base;
> -	int id = *nextid;
> +	unsigned int base = idr->idr_base;
> +	unsigned int id = *nextid;
> 
>  	if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
>  		return -EINVAL;
To verify this patch, the following is a sanity test case:

# tc qdisc delete dev $link ingress > /dev/null 2>&1;
# tc qdisc add dev $link ingress;
# tc filter add dev $link prio 1 protocol ip handle 0x80000001 parent ffff: flower skip_hw src_mac e4:11:0:0:0:2 dst_mac e4:12:0:0:0:2 action drop;
# tc filter show dev $link parent ffff:

filter pref 1 flower chain 0
filter pref 1 flower chain 0 handle 0x80000001
  dst_mac e4:12:00:00:00:02
  src_mac e4:11:00:00:00:02
  eth_type ipv4
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1

Please make sure the handle is the same as the user specifies.