netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: David Stevens <dlstevens@us.ibm.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	alan@lxorguk.ukuu.org.uk, davem@davemloft.net,
	netdev@vger.kernel.org, netdev-owner@vger.kernel.org,
	Eric Sesterhenn <snakebyte@gmx.de>
Subject: Re: [PATCH] net: fix /proc/net/snmp as memory corruptor
Date: Sun, 09 Nov 2008 09:25:54 +0100	[thread overview]
Message-ID: <49169E92.8080802@cosmosbay.com> (raw)
In-Reply-To: <OF3DE3524D.66B1A705-ON882574FB.00729B02-882574FB.0073D449@us.ibm.com>

David Stevens a écrit :
>> If you are not sure what I am talking about, then you should probably
>> not use static variables at all. I found this fix quite obvious...
> 
>         Actually, I didn't realize "out" was static -- was looking at just
> the patch, and obviously missing your point.
>         I don't have a problem with 16 ints on the stack (or shorts, as
> you pointed out)-- I didn't want the data on the stack, which may be
> 64-bit ints. In your patch, you're collecting all of it on the stack
> (doubling its size).
>         If there is no interlocking at a higher layer (and I haven't 
> looked
> at this in a long time...) (ie, exclusive opens), then I agree, it
> shouldn't be static.
> 
>         Why not just that? (ie, add count=0 as Alexey did and remove the
> static qualifier from "out")

Well, this patch also saves 143+64 bytes because of the cleanup.

before :
# size net/ipv4/proc.o
   text    data     bss     dec     hex filename
   5191      16      64    5271    1497 net/ipv4/proc.o

After :
# size net/ipv4/proc.o
   text    data     bss     dec     hex filename
   5048      16       0    5064    13c8 net/ipv4/proc.o

1) bug correction from Alexey
2) bug correction about using a static array without exclusion
3) minor cleanup to save 143 bytes of text and 64 bytes of data
4) save thousand of cpu cycles if many possible cpus.
5) Actually works, even if using 128 bytes on stack, in a function
   that only calls seq_printf() (we are not in the network stack)




  reply	other threads:[~2008-11-09  8:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-08  0:22 seq_read bugs with ipmr Eric Sesterhenn
2008-11-08  1:02 ` Alexey Dobriyan
2008-11-08  2:52   ` Alexey Dobriyan
2008-11-08  3:36     ` [PATCH] net: fix /proc/net/snmp as memory corruptor Alexey Dobriyan
2008-11-08  5:53       ` Eric Dumazet
2008-11-08  6:22         ` Alexey Dobriyan
2008-11-08  6:42         ` Alexey Dobriyan
2008-11-08  9:48           ` Eric Sesterhenn
2008-11-08 19:53         ` David Stevens
2008-11-08 20:46           ` Eric Dumazet
2008-11-08 21:05             ` David Stevens
2008-11-09  8:25               ` Eric Dumazet [this message]
2008-11-11  5:43         ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49169E92.8080802@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=adobriyan@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davem@davemloft.net \
    --cc=dlstevens@us.ibm.com \
    --cc=netdev-owner@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=snakebyte@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).