All of lore.kernel.org
 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: 30+ 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  5:45 ` Andres Salomon
2010-11-12  7:48 ` Milton Miller
2010-11-12  7:48   ` Milton Miller
2010-11-12  8:27   ` Andres Salomon
2010-11-12  8:27     ` Andres Salomon
2010-11-14  9:50     ` Ingo Molnar
2010-11-15  4:21       ` H. Peter Anvin
2010-11-15  4:21         ` H. Peter Anvin
2010-11-15  7:02         ` Ingo Molnar
2010-11-15  7:02           ` Ingo Molnar
2010-11-15 17:43           ` H. Peter Anvin
2010-11-15 17:43             ` H. Peter Anvin
2010-11-17  6:12             ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v3) Andres Salomon
2010-11-17  6:12               ` Andres Salomon
2010-11-29 23:39               ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v4) Andres Salomon
2010-12-16  2:58                 ` [tip:x86/olpc] x86, olpc: Speed up device tree creation during boot tip-bot for Andres Salomon
2010-11-18  8:34             ` [PATCH 3/3] x86: OLPC: speed up device tree creation during boot (v2) Ingo Molnar
2010-11-18  8:34               ` Ingo Molnar
2010-11-18 11:02               ` Michael Ellerman
2010-11-18 11:02                 ` Michael Ellerman
2010-11-18 15:04                 ` H. Peter Anvin
2010-11-18 15:04                   ` H. Peter Anvin
2010-11-18 17:41                   ` Andres Salomon
2010-11-18 17:41                     ` Andres Salomon
2010-11-18 17:48                     ` H. Peter Anvin
2010-11-18 17:48                       ` H. Peter Anvin
2010-11-19 20:24                       ` Andres Salomon
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.