Re: source line numbers with x86_64 modules? [Was: Re: [patch] measurements, numbers about CONFIG_OPTIMIZE_INLINING=y impact]

2009-01-11 Thread Andreas Dilger
On Jan 10, 2009  16:15 -0500, Theodore Ts'o wrote:
 In my experience, there are very few kernel versions and hardware for
 which kdump works.  I've talked to the people who have to make kdump
 work, and every 12-18 months, with a new set of enterprise kernels
 comes out, they have to go and fix kdump so it works again for the set
 of hardware that they care about, and for the kernel version involved.

I'm sad that netconsole/netdump never made it big.  It was fairly useful,
and extending the eth drivers to add the polling mode was trivial to do.
We were using that for a few years, but it got replaced by kdump and it
appears to be less usable IMHO.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning

2009-01-11 Thread David Woodhouse
On Sat, 2009-01-10 at 04:02 +0100, Andi Kleen wrote:
 Long term that problem will hopefully disappear, as gcc learns to do cross
 source file inlining (like a lot of other compilers already do)

We've already been able to get GCC doing this for the kernel, in fact
(the --combine -fwhole-program stuff I was working on a while back).

It gives an interesting size reduction, especially in file systems and
other places where we tend to have functions with a single call site...
but in a different file.

Linus argues that we don't want that kind of inlining because it harms
debuggability, but that isn't _always_ true. Sometimes you weren't going
to get a backtrace if something goes wrong _anyway_. And even if the
size reduction doesn't necessarily give a corresponding performance
improvement, we might not care. In the embedded world, size does matter
too. And the numbers are such that you can happily keep debuginfo for
the shipped kernel builds and postprocess any backtraces you get. Just
as we can for distros.


In general, I would much prefer being able to trust the compiler, rather
than disabling its heuristics completely. We might not be able to trust
it right now, but we should be working towards that state. Not just
declaring that we know best, even though _sometimes_ we do.

I think we should:

  - Unconditionally have 'inline' meaning 'always_inline'. If we say it,
we should mean it.

  - Resist the temptation to use -fno-inline-functions. Allow GCC to
inline other things if it wants to.

  - Reduce the number of unnecessary 'inline' markers, and have a policy
that the use of 'inline' should be accompanied by either a GCC PR#
or an explanation of why we couldn't reasonably have expected GCC to
get this particular case right.

  - Have a similar policy of PR# or explanation for 'uninline' too.

I don't think we should just give up on GCC ever getting it right. That
way lies madness. As we've often found in the past. 

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: source line numbers with x86_64 modules? [Was: Re: [patch] measurements, numbers about CONFIG_OPTIMIZE_INLINING=y impact]

2009-01-11 Thread Mike Snitzer
On Sun, Jan 11, 2009 at 5:11 AM, Andreas Dilger adil...@sun.com wrote:
 On Jan 10, 2009  16:15 -0500, Theodore Ts'o wrote:
 In my experience, there are very few kernel versions and hardware for
 which kdump works.  I've talked to the people who have to make kdump
 work, and every 12-18 months, with a new set of enterprise kernels
 comes out, they have to go and fix kdump so it works again for the set
 of hardware that they care about, and for the kernel version involved.

 I'm sad that netconsole/netdump never made it big.  It was fairly useful,
 and extending the eth drivers to add the polling mode was trivial to do.
 We were using that for a few years, but it got replaced by kdump and it
 appears to be less usable IMHO.

Less usable in terms of ease of configuration and use?  Or
reliability?  In practice netdump's non-interrupt-driven polling mode
proved fairly reliable but it seems to me kdump provides more reliable
dumping than netdump.  Because you're performing the kdump from a
reliable new dump kernel (provided the initial kexec transition to the
dump kernel works properly).

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] btrfs squashfs: Move btrfs and squashfsto's magic number to linux/magic.h

2009-01-11 Thread Qinghuang Feng
Use the standard magic.h for btrfs and squashfs.

Signed-off-by: Qinghuang Feng qhfeng.ker...@gmail.com
---
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 0a14b49..7256cf2 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -38,6 +38,7 @@
 #include linux/namei.h
 #include linux/miscdevice.h
 #include linux/version.h
+#include linux/magic.h
 #include compat.h
 #include ctree.h
 #include disk-io.h
@@ -51,7 +52,6 @@
 #include export.h
 #include compression.h
 
-#define BTRFS_SUPER_MAGIC 0x9123683E
 
 static struct super_operations btrfs_super_ops;
 
diff --git a/fs/squashfs/squashfs_fs.h b/fs/squashfs/squashfs_fs.h
index 6840da1..283daaf 100644
--- a/fs/squashfs/squashfs_fs.h
+++ b/fs/squashfs/squashfs_fs.h
@@ -26,7 +26,6 @@
 #define SQUASHFS_CACHED_FRAGMENTS  CONFIG_SQUASHFS_FRAGMENT_CACHE_SIZE
 #define SQUASHFS_MAJOR 4
 #define SQUASHFS_MINOR 0
-#define SQUASHFS_MAGIC 0x73717368
 #define SQUASHFS_START 0
 
 /* size of metadata (inode and directory) blocks */
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index a0466d7..071df5b 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -35,6 +35,7 @@
 #include linux/init.h
 #include linux/module.h
 #include linux/zlib.h
+#include linux/magic.h
 
 #include squashfs_fs.h
 #include squashfs_fs_sb.h
diff --git a/include/linux/magic.h b/include/linux/magic.h
index 439f6f3..0b4df7e 100644
--- a/include/linux/magic.h
+++ b/include/linux/magic.h
@@ -10,11 +10,13 @@
 #define SYSFS_MAGIC0x62656572
 #define SECURITYFS_MAGIC   0x73636673
 #define TMPFS_MAGIC0x01021994
+#define SQUASHFS_MAGIC 0x73717368
 #define EFS_SUPER_MAGIC0x414A53
 #define EXT2_SUPER_MAGIC   0xEF53
 #define EXT3_SUPER_MAGIC   0xEF53
 #define XENFS_SUPER_MAGIC  0xabba1974
 #define EXT4_SUPER_MAGIC   0xEF53
+#define BTRFS_SUPER_MAGIC  0x9123683E
 #define HPFS_SUPER_MAGIC   0xf995e849
 #define ISOFS_SUPER_MAGIC  0x9660
 #define JFFS2_SUPER_MAGIC  0x72b6

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


gcc inlining heuristics was Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning

2009-01-11 Thread Andi Kleen
On Sun, Jan 11, 2009 at 11:25:32AM -0800, Linus Torvalds wrote:
 
 
 On Sun, 11 Jan 2009, Andi Kleen wrote:
  
  The proposal was to use -fno-inline-functions-called-once (but 
  the resulting numbers were not promising)
 
 Well, the _optimal_ situation would be to not need it, because gcc does a 
 good job without it. That implies trying to find a better balance between 
 worth it and causes problems. 
 
 Rigth now, it does sound like gcc simply doesn't try to balance AT ALL, or 
 balances only when we add some very version-specific random options (ie 
 the stack-usage one). 

The gcc 4.3 inliner takes stack growth into account by default (without
any special options). I experimented a bit with it when that
was introduced and found the default thresholds are too large for the kernel 
and don't change the checkstack.pl picture much.

I asked back then and was told --param large-stack-frame
is expected to be a reasonable stable --param (as much as these can
be) and I did a patch to lower it, but I couldn't get myself
to actually submit it [if you really want it I can send it]. 

But of course that only helps for gcc 4.3+, older gccs would need
a different workaround.

On the other hand (my personal opinion, not shared by everyone) is 
that the ioctl switch stack issue is mostly only a problem with 4K 
stacks and in the rare cases when I still run 32bit kernels
I never set that option because I consider it russian roulette
(because there undoutedly dangerous dynamic stack growth cases that 
checkstack.pl doesn't flag) 

 And even those options don't actually make much 
 sense - yes, they balance things, but they don't do it in a sensible 
 manner.
 
 For example: stack usage is undeniably a problem (we've hit it over and 
 over again), but it's not about stack must not be larger than X bytes. 
 
 If the call is done unconditionally, then inlining _one_ function will 
 grow the static stack usage of the function we inline into, but it will 
 _not_ grow the dynamic stack usage one whit - so deciding to not inline 
 because of stack usage is pointless.

Don't think the current inliner takes that into account from
a quick look at the sources, although it probably could.  
Maybe Honza can comment.

But even if it did it could only do that for a single file,
but if the function is in a different file gcc doesn't
have the information (unless you run with David's --combine hack).
This means the kernel developers have to do it anyways.

On the other hand I'm not sure it's that big a problem. Just someone
should run make checkstack occasionally and add noinlines to large
offenders.

-Andi

[keep quote for Honza's benefit]

 
 See? So stop inlining when you hit a stack limit IS THE WRONG THING TO 
 DO TOO! Because it just means that the compiler continues to do bad 
 inlining decisions until it hits some magical limit - but since the 
 problem isn't the static stack size of any _single_ function, but the 
 combined stack size of a dynamic chain of them, that's totally idiotic. 
 You still grew the dynamic stack, and you have no way of knowing by how 
 much - the limit on the static one simply has zero bearing what-so-ever on 
 the dynamic one.
 
 So no, limit static stack usage is not a good option, because it stops 
 inlining when it doesn't matter (single unconditional call), and doesn't 
 stop inlining when it might (lots of sequential calls, in a deep chain).
 
 The other alternative is to let gcc do what it does, but 
 
  (a) remove lots of unnecessary 'inline's. And we should likely do this 
  regardless of any -fno-inline-functions-called-once issues.
 
  (b) add lots of 'noinline's to avoid all the cases where gcc screws up so 
  badly that it's either a debugging disaster or an actual correctness 
  issue.
 
 The problem with (b) is that it's a lot of hard thinking, and debugging 
 disasters always happen in code that you didn't realize would be a problem 
 (because if you had, it simply wouldn't be the debugging issue it is).
 
   Linus
 

-- 
a...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: gcc inlining heuristics was Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning

2009-01-11 Thread David Woodhouse
On Sun, 2009-01-11 at 21:14 +0100, Andi Kleen wrote:
 
 On the other hand (my personal opinion, not shared by everyone) is 
 that the ioctl switch stack issue is mostly only a problem with 4K 
 stacks and in the rare cases when I still run 32bit kernels
 I never set that option because I consider it russian roulette
 (because there undoutedly dangerous dynamic stack growth cases that 
 checkstack.pl doesn't flag) 

Isn't the ioctl switch stack issue a separate GCC bug?

It was/is assigning assigning separate space for local variables which
are mutually exclusive. So instead of the stack footprint of the
function with the switch() being equal to the largest individual stack
size of all the subfunctions, it's equal to the _sum_ of the stack sizes
of the subfunctions. Even though it'll never use them all at the same
time.

Without that bug, it would have been harmless to inline them all.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: gcc inlining heuristics was Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning

2009-01-11 Thread Linus Torvalds


On Sun, 11 Jan 2009, Andi Kleen wrote:
 
 Was -- i think that got fixed in gcc. But again only in newer versions.

I doubt it. People have said that about a million times, it has never 
gotten fixed, and I've never seen any actual proof.

I think that what got fixed was that gcc now at least re-uses stack slots 
for temporary spills. But only for things that fit in registers - not if 
you actually had variables that are big enough to be of type MEM. And the 
latter is what tends to eat stack-space (ie structures etc on stack).

But hey, maybe it really did get fixed. But the last big stack user wasn't 
that long ago, and I saw it and I have a pretty recent gcc (gcc-4.3.2 
right now, it could obviously have been slightly older back a few months 
ago).

Linus
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] btrfs: MAINTAINERS entry

2009-01-11 Thread Joe Perches
Now that btrfs is in mainline, perhaps
a maintainers entry is appropriate?

Perhaps:

diff --git a/MAINTAINERS b/MAINTAINERS
index 6f65a26..138a54c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1021,6 +1021,14 @@ M:   m...@bu3sch.de
 W: http://bu3sch.de/btgpio.php
 S: Maintained
 
+BTRFS FILE SYSTEM
+P: Chris Mason
+M: chris.ma...@oracle.com
+L: linux-btrfs@vger.kernel.org
+W: http://btrfs.wiki.kernel.org/
+T: git kernel.org:/pub/scm/linux/kernel/git/mason/btrfs-unstable.git
+S: Maintained
+
 BTTV VIDEO4LINUX DRIVER
 P: Mauro Carvalho Chehab
 M: mche...@infradead.org


--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html