netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* using rhashtable in inethash
@ 2014-08-25 23:12 David Miller
  2014-08-25 23:33 ` Thomas Graf
  2014-08-25 23:35 ` Eric Dumazet
  0 siblings, 2 replies; 6+ messages in thread
From: David Miller @ 2014-08-25 23:12 UTC (permalink / raw)
  To: tgraf; +Cc: eric.dumazet, netdev


During the Networking Workshop I mentioned converting the inet hash
tables over to rhashtable so that we don't allocate this insanely
large hash table at boot time which goes largely unused.

I took a quick look at this last night and the only thing we really
need is the addition of a set of rhashtable interfaces which use
NULLs lists, as the inet hashtables currently require.

Also, I noticed in the netlink changes this really expensive
synchronize_net() added to netlink_release(), is that _really_
necessary?

That's really expensive and my impression was that such a sync is only
needed during hash table resizing, not when getting rid of objects
that we in an rhashtable.

Thomas?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: using rhashtable in inethash
  2014-08-25 23:12 using rhashtable in inethash David Miller
@ 2014-08-25 23:33 ` Thomas Graf
  2014-08-25 23:36   ` David Miller
  2014-08-25 23:35 ` Eric Dumazet
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Graf @ 2014-08-25 23:33 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, paulmck

On 08/25/14 at 04:12pm, David Miller wrote:
> During the Networking Workshop I mentioned converting the inet hash
> tables over to rhashtable so that we don't allocate this insanely
> large hash table at boot time which goes largely unused.

> I took a quick look at this last night and the only thing we really
> need is the addition of a set of rhashtable interfaces which use
> NULLs lists, as the inet hashtables currently require.

Eric brought this up as well last week. I see no problem using
a non-NULL token as the default to identify the end of the list.

We had a quick sitdown with Paul E. McKenney and came up with
some ideas that should allow doing that even with resizes taking
place by using the hash of the entry as the token.

We also discussed the possibility to do the resizing outside of
the insert/remove context and move it to a worker thread using
per bucket locks. We think that we may have found something that
might work and I will give that a shot. It will allow to use a
resizing rhashtable with the insert/remove being in atomic
context which I believe is needed for the inet cache.

> Also, I noticed in the netlink changes this really expensive
> synchronize_net() added to netlink_release(), is that _really_
> necessary?
>
> 
> That's really expensive and my impression was that such a sync is only
> needed during hash table resizing, not when getting rid of objects
> that we in an rhashtable.

The reason I added added the sync is because I could not see how
else to prevent the sock_put() in netlink_release() to release
socket memoray, specifically the embedded rhash_head, that is
possibly being accessed in a RCU protected reader traversing the
bucket. Such a reader would not hold a reference to the socket.
I may be missing something though and I'm happy to change this for
something better.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: using rhashtable in inethash
  2014-08-25 23:12 using rhashtable in inethash David Miller
  2014-08-25 23:33 ` Thomas Graf
@ 2014-08-25 23:35 ` Eric Dumazet
  2014-08-25 23:42   ` Thomas Graf
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-08-25 23:35 UTC (permalink / raw)
  To: David Miller; +Cc: tgraf, netdev, Paul E. McKenney, John Fastabend

On Mon, 2014-08-25 at 16:12 -0700, David Miller wrote:
> During the Networking Workshop I mentioned converting the inet hash
> tables over to rhashtable so that we don't allocate this insanely
> large hash table at boot time which goes largely unused.
> 
> I took a quick look at this last night and the only thing we really
> need is the addition of a set of rhashtable interfaces which use
> NULLs lists, as the inet hashtables currently require.
> 
> Also, I noticed in the netlink changes this really expensive
> synchronize_net() added to netlink_release(), is that _really_
> necessary?
> 
> That's really expensive and my impression was that such a sync is only
> needed during hash table resizing, not when getting rid of objects
> that we in an rhashtable.
> 
> Thomas?

After the workshop we had a discussion with Paul McKenney, Thomas, and
John where we discussed all this.

An other issue we raised was that the grow should happen in process
context, while inserts and deletes should happen from softirq.

We also mentioned the need of a per bucket spinlock, and keep the mutex
only to protect the resizes. (Or an array of spinlocks as used by TCP)

Paul gave a lot of ideas and this seems feasible.

Thomas agreed to work on all this. If not I can do this myself.

(Note that conntrack also uses the same nulls rcu hash tables)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: using rhashtable in inethash
  2014-08-25 23:33 ` Thomas Graf
@ 2014-08-25 23:36   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-08-25 23:36 UTC (permalink / raw)
  To: tgraf; +Cc: eric.dumazet, netdev, paulmck

From: Thomas Graf <tgraf@suug.ch>
Date: Tue, 26 Aug 2014 00:33:46 +0100

> The reason I added added the sync is because I could not see how
> else to prevent the sock_put() in netlink_release() to release
> socket memoray, specifically the embedded rhash_head, that is
> possibly being accessed in a RCU protected reader traversing the
> bucket. Such a reader would not hold a reference to the socket.
> I may be missing something though and I'm happy to change this for
> something better.

Ok, this is going to be a tree-wide issue where we try to use
rhashtable with sockets of any type.

Perhaps we're overdue for RCU freeing of sockets, but that added
noticable latency last time I tried it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: using rhashtable in inethash
  2014-08-25 23:35 ` Eric Dumazet
@ 2014-08-25 23:42   ` Thomas Graf
  2014-08-26  0:56     ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Graf @ 2014-08-25 23:42 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Paul E. McKenney, John Fastabend

On 08/25/14 at 04:35pm, Eric Dumazet wrote:
> Thomas agreed to work on all this. If not I can do this myself.

I already got started and will share the code as soon as I have
something semi functional.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: using rhashtable in inethash
  2014-08-25 23:42   ` Thomas Graf
@ 2014-08-26  0:56     ` Paul E. McKenney
  0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2014-08-26  0:56 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Eric Dumazet, David Miller, netdev, John Fastabend

On Tue, Aug 26, 2014 at 12:42:31AM +0100, Thomas Graf wrote:
> On 08/25/14 at 04:35pm, Eric Dumazet wrote:
> > Thomas agreed to work on all this. If not I can do this myself.
> 
> I already got started and will share the code as soon as I have
> something semi functional.

Very much looking forward to seeing it!

							Thanx, Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-08-26  0:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-25 23:12 using rhashtable in inethash David Miller
2014-08-25 23:33 ` Thomas Graf
2014-08-25 23:36   ` David Miller
2014-08-25 23:35 ` Eric Dumazet
2014-08-25 23:42   ` Thomas Graf
2014-08-26  0:56     ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).