[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150323083319.GB16023@casper.infradead.org>
Date: Mon, 23 Mar 2015 08:33:19 +0000
From: Thomas Graf <tgraf@...g.ch>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <eric.dumazet@...il.com>,
Patrick McHardy <kaber@...sh.net>,
Josh Triplett <josh@...htriplett.org>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
netdev@...r.kernel.org
Subject: Re: [v2 PATCH 7/10] rhashtable: Disable automatic shrinking
On 03/23/15 at 11:09am, Herbert Xu wrote:
> On Sun, Mar 22, 2015 at 12:17:55PM +0000, Thomas Graf wrote:
> >
> > This is misleading. I agree that unconditional shrinking is dangerous.
> > Shrinking was an optional feature disabled by default before. The
>
> How was shrinking disabled before? AFAICS it always kicked in at
> 30%.
Before Daniel removed the indirection due to all callers enabling
shrinking by default ;-) It was clear that some future users
eventually would not want shrinking and thus require a conditional.
> > inlining enabled it by default for all users. What is the benefit of
> > requiring this logic outside of rhashtable over just adding a flag to
> > enable shrinking at 30% utilization?
>
> That would be adding an extra branch on the fast-path for an
> operation which almost nobody needs.
I don't get why almost nobody would want shrinking. I agree that for
tables like TCP hash tables, once you have grown you want to keep that
table size because the load is likely to come back. But we will also
have lots of users such as the Netlink socket with a table per protocol
where not shrinking results in giving the user the ability to waste
memory indefinitely for no gain.
I'm not claiming you always want shrinking but what gain is there by
removing integrated support? Can you show numbers that the additional
branch actually hurts?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists