[Cake] lockup with multiple cake instances on 3.16.7

[Pete Heist]

> On Jan 31, 2019, at 3:53 PM, Toke Høiland-Jørgensen <toke at redhat.com> wrote:
> 
> Well, the backtrace is definitely hanging on that lock in
> gnet_stats_start_copy_compat(). If it's not related to the CAKE logging,
> I guess it must be a bug in the upstream kernel; which we probably can't
> fix from the cake side anyway.
> 
> I don't suppose you can reproduce this on a newer kernel?

So far it works fine on 3.10.107 (with mipsel EdgeOS build) and 4.9.0-8 amd64, haven’t tried anything in between, but…

printk tells me that it’s not locking up in cake_dump_class_stats, but after a failure in cake_dump_stats. When “tc qdisc” is run after adding the fifth cake instance, line 2974 is failing:

                PUT_TSTAT_U32(TARGET_US,
                              ktime_to_us(ns_to_ktime(b->cparams.target)));

So the call to nla_put_u32 returns nonzero. Then it ends up at nla_put_failure where nla_nest_cancel is called. The function returns, but the lock is not being released by the kernel in the failure case. The following patch “fixes it”:

diff --git a/sch_cake.c b/sch_cake.c
index 3a26db0..ae3e16c 100644
--- a/sch_cake.c
+++ b/sch_cake.c
@@ -3010,6 +3010,7 @@ static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 
 nla_put_failure:
        nla_nest_cancel(d->skb, stats);
+       sch_tree_unlock(sch);
        return -1;
 }

Two questions:

1) Why is nla_put_u32 suddenly failing for TARGET_US after adding five cake instances?

2) Is calling sch_tree_unlock the right thing to do in the failure case, or am I working around a kernel bug, and doing something that would fail in other kernels?