Re: [v3 PATCH 0/2] rhashtable: rhashtable with duplicate objects

From: Johannes Berg Date: Mon, 19 Sep 2016 13:32:12 +0200 > On Mon, 2016-09-19 at 13:03 +0200, Johannes Berg wrote: >> On Mon, 2016-09-19 at 18:58 +0800, Herbert Xu wrote: >> > >> > v3 fixes a bug in the remove path that causes the element count >> > to decrease when

Re: [v3 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 13:03 +0200, Johannes Berg wrote: > On Mon, 2016-09-19 at 18:58 +0800, Herbert Xu wrote: > > > > v3 fixes a bug in the remove path that causes the element count > > to decrease when it shouldn't, leading to a gigantic hash table > > when it underflows. > > > Ok, with the

Re: [v3 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 18:58 +0800, Herbert Xu wrote: > v3 fixes a bug in the remove path that causes the element count > to decrease when it shouldn't, leading to a gigantic hash table > when it underflows. > Ok, with the BUG_ON() thrown in, this works in the test that was failing before. I'll

[v3 PATCH 0/2] rhashtable: rhashtable with duplicate objects

v3 fixes a bug in the remove path that causes the element count to decrease when it shouldn't, leading to a gigantic hash table when it underflows. v2 contains a reworked insertion slowpath to ensure that the spinlock for the table we're inserting into is taken. This series contains two patches.

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 18:48 +0800, Herbert Xu wrote: > On Mon, Sep 19, 2016 at 12:10:27PM +0200, Johannes Berg wrote: > > > > Btw, for debug I put > > > > BUG_ON(atomic_read(>nelems) < 0); > > > > after the atomic_dec() in __rhashtable_remove_fast_one(). That > > makes > > the kernel crash

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, Sep 19, 2016 at 12:10:27PM +0200, Johannes Berg wrote: > Btw, for debug I put > > BUG_ON(atomic_read(>nelems) < 0); > > after the atomic_dec() in __rhashtable_remove_fast_one(). That makes > the kernel crash instantly on the buggy code, and I just have to run a > single test

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

Btw, for debug I put BUG_ON(atomic_read(>nelems) < 0); after the atomic_dec() in __rhashtable_remove_fast_one(). That makes the kernel crash instantly on the buggy code, and I just have to run a single test ("wpas_ctrl_interface_add_many") to get there. johannes

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 12:02 +0200, Johannes Berg wrote: > On Mon, 2016-09-19 at 11:54 +0200, Johannes Berg wrote: > > > > > > > > > > > The stack trace is useless, but my other annotation showed that > > > the > > > table's nelems *underflowed* to -1, so now the worker will > > > continue > > >

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 11:54 +0200, Johannes Berg wrote: > > > > The stack trace is useless, but my other annotation showed that the > > table's nelems *underflowed* to -1, so now the worker will continue > > to try to grow it forever. > > > > And this *was* actually a case of duplication,

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

> The stack trace is useless, but my other annotation showed that the > table's nelems *underflowed* to -1, so now the worker will continue > to try to grow it forever. > And this *was* actually a case of duplication, afaict, since it was multiple virtual interfaces on the same device all

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 17:34 +0800, Herbert Xu wrote: > On Mon, Sep 19, 2016 at 11:27:24AM +0200, Johannes Berg wrote: > > > > > > I have a feeling there's a bug with ht->nelems, since the crash is > > always in the grow worker, but I haven't quite put my finger on it > > yet. > > Can you show

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 17:34 +0800, Herbert Xu wrote: > On Mon, Sep 19, 2016 at 11:27:24AM +0200, Johannes Berg wrote: > > > > > > I have a feeling there's a bug with ht->nelems, since the crash is > > always in the grow worker, but I haven't quite put my finger on it > > yet. > > Can you show

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, Sep 19, 2016 at 11:27:24AM +0200, Johannes Berg wrote: > > I have a feeling there's a bug with ht->nelems, since the crash is > always in the grow worker, but I haven't quite put my finger on it yet. Can you show me a stack trace? Thanks, -- Email: Herbert Xu

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 11:27 +0200, Johannes Berg wrote: > >  > I have a feeling there's a bug with ht->nelems, since the crash is > always in the grow worker, but I haven't quite put my finger on it > yet. > Btw, I should not actually get into the duplicate case here. johannes

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

> What does your test suite actually do? Is it something that I > can run without special hardware? Yes, it's pretty simple - it spins up a number of VMs with hwsim and just runs a lot of tests: https://w1.fi/cgit/hostap/tree/tests/hwsim I've attached a kernel .config you can use for it. I'm

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, Sep 19, 2016 at 11:15:30AM +0200, Johannes Berg wrote: > On Mon, 2016-09-19 at 16:40 +0800, Herbert Xu wrote: > > > I've tested the rhlist code with test_rhashtable but I haven't > > tested the mac80211 conversion.  So please give it a go and see > > if it still works. > > This is still

Re: [v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, 2016-09-19 at 16:40 +0800, Herbert Xu wrote: > I've tested the rhlist code with test_rhashtable but I haven't > tested the mac80211 conversion.  So please give it a go and see > if it still works. This is still running out of memory on my test suite. Somehow I don't see kmemleak kicking

Re: [PATCH 0/2] rhashtable: rhashtable with duplicate objects

> > I take that back. I think it's leaking memory - my tests never used > > to run out of memory, but now they eventually do. > > > I'll try to figure out more. > > Interesting.  The kernel test robot found a bug in the insertion > slowpath where we end up inserting without taking the inner

[v2 PATCH 0/2] rhashtable: rhashtable with duplicate objects

v2 contains a reworked insertion slowpath to ensure that the spinlock for the table we're inserting into is taken. This series contains one two patches. The first adds the rhlist interface and the second converts mac80211 to use it. If this works out I'll then proceed to convert the other

Re: [PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Mon, Sep 19, 2016 at 10:25:18AM +0200, Johannes Berg wrote: > > > Yes, it's passing all the wpa_supplicant tests, so > > > > Acked-by: Johannes Berg > > > > I take that back. I think it's leaking memory - my tests never used to > run out of memory, but now they

Re: [PATCH 0/2] rhashtable: rhashtable with duplicate objects

> Yes, it's passing all the wpa_supplicant tests, so > > Acked-by: Johannes Berg > I take that back. I think it's leaking memory - my tests never used to run out of memory, but now they eventually do. I'll try to figure out more. johannes

Re: [PATCH 0/2] rhashtable: rhashtable with duplicate objects

> OK, it's finally ready now. > > This series contains one two patches.  The first adds the rhlist > interface and the second converts mac80211 to use it.  If this works > out I'll then proceed to convert the other insecure_elasticity > users over to this. Thanks a lot Herbert! > I've tested

[PATCH 0/2] rhashtable: rhashtable with duplicate objects

On Fri, Aug 05, 2016 at 12:50:33PM +0200, Johannes Berg wrote: > > My plan is to build support for this directly into rhashtable. > > So I'm adding a struct rhlist_head that would be used in place > > of rhash_head for these cases and it'll carry an extra pointer > > for the list of identical