lkml.org 
[lkml]   [2016]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [RFC PATCH v3 2/7] proc: Reduce cache miss in {snmp,netstat}_seq_show
Date
Hi Marcelo

sorry for the late, just came back from a vacation.

On 9/14/16 7:55 PM, Marcelo wrote:
> Hi Jia,
>
> On Wed, Sep 14, 2016 at 01:58:42PM +0800, hejianet wrote:
>> Hi Marcelo
>>
>>
>> On 9/13/16 2:57 AM, Marcelo wrote:
>>> On Fri, Sep 09, 2016 at 02:33:57PM +0800, Jia He wrote:
>>>> This is to use the generic interface snmp_get_cpu_field{,64}_batch to
>>>> aggregate the data by going through all the items of each cpu sequentially.
>>>> Then snmp_seq_show and netstat_seq_show are split into 2 parts to avoid build
>>>> warning "the frame size" larger than 1024 on s390.
>>> Yeah about that, did you test it with stack overflow detection?
>>> These arrays can be quite large.
>>>
>>> One more below..
>> Do you think it is acceptable if the stack usage is a little larger than 1024?
>> e.g. 1120
>> I can't find any other way to reduce the stack usage except use "static" before
>> unsigned long buff[TCP_MIB_MAX]
>>
>> PS. sizeof buff is about TCP_MIB_MAX(116)*8=928
>> B.R.
> That's pretty much the question. Linux has the option on some archs to
> run with 4Kb (4KSTACKS option), so this function alone would be using
> 25% of it in this last case. While on x86_64, it uses 16Kb (6538b8ea886e
> ("x86_64: expand kernel stack to 16K")).
>
> Adding static to it is not an option as it actually makes the variable
> shared amongst the CPUs (and then you have concurrency issues), plus the
> fact that it's always allocated, even while not in use.
>
> Others here certainly know better than me if it's okay to make such
> usage of the stach.
What about this patch instead?
It is a trade-off. I split the aggregation process into 2 parts, it will
increase the cache miss a little bit, but it can reduce the stack usage.
After this, stack usage is 672bytes
objdump -d vmlinux | ./scripts/checkstack.pl ppc64 | grep seq_show
0xc0000000007f7cc0 netstat_seq_show_tcpext.isra.3 [vmlinux]:672

diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index c6ee8a2..cc41590 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -486,22 +486,37 @@ static const struct file_operations snmp_seq_fops = {
*/
static int netstat_seq_show_tcpext(struct seq_file *seq, void *v)
{
- int i;
- unsigned long buff[LINUX_MIB_MAX];
+ int i, c;
+ unsigned long buff[LINUX_MIB_MAX/2 + 1];
struct net *net = seq->private;

- memset(buff, 0, sizeof(unsigned long) * LINUX_MIB_MAX);
+ memset(buff, 0, sizeof(unsigned long) * (LINUX_MIB_MAX/2 + 1));

seq_puts(seq, "TcpExt:");
for (i = 0; snmp4_net_list[i].name; i++)
seq_printf(seq, " %s", snmp4_net_list[i].name);

seq_puts(seq, "\nTcpExt:");
- snmp_get_cpu_field_batch(buff, snmp4_net_list,
- net->mib.net_statistics);
- for (i = 0; snmp4_net_list[i].name; i++)
+ for_each_possible_cpu(c) {
+ for (i = 0; i < LINUX_MIB_MAX/2; i++)
+ buff[i] += snmp_get_cpu_field(
+ net->mib.net_statistics,
+ c, snmp4_net_list[i].entry);
+ }
+ for (i = 0; i < LINUX_MIB_MAX/2; i++)
seq_printf(seq, " %lu", buff[i]);

+ memset(buff, 0, sizeof(unsigned long) * (LINUX_MIB_MAX/2 + 1));
+ for_each_possible_cpu(c) {
+ for (i = LINUX_MIB_MAX/2; snmp4_net_list[i].name; i++)
+ buff[i - LINUX_MIB_MAX/2] += snmp_get_cpu_field(
+ net->mib.net_statistics,
+ c,
+ snmp4_net_list[i].entry);
+ }
+ for (i = LINUX_MIB_MAX/2; snmp4_net_list[i].name; i++)
+ seq_printf(seq, " %lu", buff[i - LINUX_MIB_MAX/2]);
+
return 0;
}

>>>> +static int netstat_seq_show_ipext(struct seq_file *seq, void *v)
>>>> +{
>>>> + int i;
>>>> + u64 buff64[IPSTATS_MIB_MAX];
>>>> + struct net *net = seq->private;
>>>> seq_puts(seq, "\nIpExt:");
>>>> for (i = 0; snmp4_ipextstats_list[i].name != NULL; i++)
>>>> seq_printf(seq, " %s", snmp4_ipextstats_list[i].name);
>>>> seq_puts(seq, "\nIpExt:");
>>> You're missing a memset() call here.
> Not sure if you missed this one or not..
indeed, thanks
B.R.
Jia
> Thanks,
> Marcelo

\
 
 \ /
  Last update: 2016-09-23 00:00    [W:0.080 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site