Re: Patch: [NET]: Remove CONFIG_PROC_FS depency for pcounter inuse

[Eric Dumazet <dada1@cosmosbay.com>]

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.