[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47C945D7.7050601@cosmosbay.com>
Date: Sat, 01 Mar 2008 13:02:31 +0100
From: Eric Dumazet <dada1@...mosbay.com>
To: Oliver Hartkopp <oliver@...tkopp.net>
CC: Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: Patch: [NET]: Remove CONFIG_PROC_FS depency for pcounter inuse
Oliver Hartkopp a écrit :
> Eric Dumazet wrote:
>> Oliver Hartkopp a écrit :
>>> Hi all,
>>>
>>> attached you'll find a patch that fixes the depency that has been
>>> introduced in commit 65f7651788e18fadb2fbb7276af935d7871e1803 ([NET]:
>>> prot_inuse cleanups and optimizations).
>>>
>>> As the inuse counters are only used by internet protocols right now,
>>> using CONFIG_INET would have been more obvious to recognize this
>>> illegal optimization here. Going a bit deeper into this problem we
>>> can see, that the pcounters are ONLY used for the internet protocols
>>> BUT initialized for ALL protocols in proto_[un|]register() in
>>> net/core/sock.c.
>>>
>>> This forces all network protocols to initialize the pcounters and
>>> therefore request dynamic percpu memory even when it is not used at all.
>>>
>>> I would suggest to
>>>
>>> 1. move the ..._inuse_[init|free]() stuff from sock.c to
>>> af_inet[|6].c and his friends
>>>
>>> OR
>>>
>>> 2. add new parameters to proto_[un|]register() like 'alloc_inuse' and
>>> 'free_inuse'
>>>
>>> My favourite sollution would be the second one but before creating a
>>> patch for one of these suggestions, i wanted to ask for your opinion
>>> or if there is any 'even nicer' idea from your side.
>>
>> Hello Oliver
>>
>> I am just coming back from hollidays.
>
> Lucky guy ;-)
>
>>
>> Last thing I did before leaving was to post a patch to correct
>> performance hit of percpu_counters in mainline. ([PATCH]
>> alloc_percpu() fails to allocate percpu data
>> http://lkml.org/lkml/2008/2/21/254 )
>>
>> Before accepting Andrew Morton claims about percpu_counters being
>> superior to pcounter, I benched them and found they were not.
>>
>> As soon as percpu_counters are not grossly inefficient, the only move
>> will be to just zap pcounter, as most people dont like it.
>>
>> Only one patch will be necessary, please dont try to hide pcounter by
>> small patches :)
>
> Hm - i followed the discussion in it's major parts but my RFC hit's the
> question whether the integration of the what-ever-per-cpu-counter
> initialisation in proto_register() and proto_unregister() is the right
> way as only the internet protocols (v4/v6) are using inuse counters
> these days.
>
> It's not about the counter implementation but the integration/usage in
> the networking subsystem.
>
> Or does your mentioned patch mean, that the added functions in
> proto_[un|]register() will also be reverted?
>
A patch will make inet use percpu_counter instead of pcounter.
Then a zap patch will delete lib/pcounter.c & include/linux/pcounter.h
I dont understand why you say CONFIG_PROC_FS is *forced*.
I can build a kernel with CONFIG_PROC_FS=n, with working INET.
--
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