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

From: Chris Mi
Date: Fri Feb 23 2018 - 20:49:48 EST


> -----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.