devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andres Salomon <dilinger@queued.net>,
	Milton Miller <miltonm@bga.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	devicetree-discuss@lists.ozlabs.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v2)
Date: Thu, 23 Dec 2010 12:57:16 +0100	[thread overview]
Message-ID: <20101223115716.GA2108@elte.hu> (raw)
In-Reply-To: <20101118083420.GC26398@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> > However, I have to vehemently object to putting them in a wider scope
> > than is otherwise necessary.  I agree that static variables should be
> > used sparsely if at all (there really are vary few uses of them that are
> > valid), but putting them in a larger scope screams "I'm used in more
> > than one function", and that is *not* a good thing.
> 
> That's why we sometimes use the (imperfect) compromise to put them in front of 
> that function, not at the top of the file.
> 
> Look at the general balance of hardship: very little harm is done (it's not a big 
> deal if a variable is only used in a single function) but having it with local 
> variables can be _really_ harmful - for example i overlooked them when i reviewed 
> this patch. I dont like important details obscured - i like them to be apparent. 
> Again, this is something that some people can parse immediately on the visual 
> level - me and many others cannot.

As an addendum, beyond my own bad experience with them, see below a recent upstream 
fix that shows the kinds of problems that overlooked function scope statics can 
cause.

	Ingo

------------->
>From 3cb50ddf97a0a1ca4c68bc12fa1e727a6b45fbf2 Mon Sep 17 00:00:00 2001
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Mon, 20 Dec 2010 15:53:18 +0000
Subject: [PATCH] Fix btrfs b0rkage

Buggered-in: 76dda93c6ae2 ("Btrfs: add snapshot/subvolume destroy
ioctl")

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Chris Mason <chris.mason@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 fs/btrfs/export.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 6f04444..659f532 100644
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -166,7 +166,7 @@ static struct dentry *btrfs_fh_to_dentry(struct super_block *sb, struct fid *fh,
 static struct dentry *btrfs_get_parent(struct dentry *child)
 {
 	struct inode *dir = child->d_inode;
-	static struct dentry *dentry;
+	struct dentry *dentry;
 	struct btrfs_root *root = BTRFS_I(dir)->root;
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;

      parent reply	other threads:[~2010-12-23 11:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-12  5:45 [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v2) Andres Salomon
2010-11-12  7:48 ` Milton Miller
     [not found]   ` <reply-olpc-3-v2-kM9DGJe42AJBDgjK7y7TUQ@public.gmane.org>
2010-11-12  8:27     ` Andres Salomon
2010-11-14  9:50       ` Ingo Molnar
     [not found]         ` <20101114095013.GB24206-X9Un+BFzKDI@public.gmane.org>
2010-11-15  4:21           ` H. Peter Anvin
     [not found]             ` <4CE0B54E.6000101-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2010-11-15  7:02               ` Ingo Molnar
     [not found]                 ` <20101115070254.GA25243-X9Un+BFzKDI@public.gmane.org>
2010-11-15 17:43                   ` H. Peter Anvin
     [not found]                     ` <4CE17133.2050101-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2010-11-17  6:12                       ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v3) Andres Salomon
2010-11-29 23:39                         ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v4) Andres Salomon
2010-11-18  8:34                       ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v2) Ingo Molnar
     [not found]                         ` <20101118083420.GC26398-X9Un+BFzKDI@public.gmane.org>
2010-11-18 11:02                           ` Michael Ellerman
2010-11-18 15:04                             ` H. Peter Anvin
     [not found]                               ` <4CE54064.6010702-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2010-11-18 17:41                                 ` Andres Salomon
     [not found]                                   ` <20101118094153.4515cbc2-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
2010-11-18 17:48                                     ` H. Peter Anvin
     [not found]                                       ` <4CE5670B.1060300-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2010-11-19 20:24                                         ` Andres Salomon
2010-12-23 11:57                         ` Ingo Molnar [this message]

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=20101223115716.GA2108@elte.hu \
    --to=mingo@elte.hu \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=dilinger@queued.net \
    --cc=grant.likely@secretlab.ca \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miltonm@bga.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.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).