Hi !
sector_t depends on CONFIG_LBD but include/config.h may not be there
thus causing interesting breakage in some places...
Here's the fix for ppc32 (problem found by Roman Zippel, other archs
need a similar fix).
diff -urN linux-2.5/include/asm-ppc/types.h linuxppc-2.5-benh/include/asm-ppc/types.h
--- linux-2.5/include/asm-ppc/types.h 2004-03-01 18:13:06.000000000 +1100
+++ linuxppc-2.5-benh/include/asm-ppc/types.h 2004-03-29 13:14:50.000000000 +1000
@@ -37,6 +37,8 @@
#ifndef __ASSEMBLY__
+#include <linux/config.h>
+
typedef signed char s8;
typedef unsigned char u8;
Benjamin Herrenschmidt <[email protected]> wrote:
>
> sector_t depends on CONFIG_LBD but include/config.h may not be there
> thus causing interesting breakage in some places...
Nasty.
> Here's the fix for ppc32 (problem found by Roman Zippel, other archs
> need a similar fix).
Three of them.
25-akpm/include/asm-s390/types.h | 2 ++
25-akpm/include/asm-sh/types.h | 2 ++
25-akpm/include/asm-x86_64/types.h | 2 ++
3 files changed, 6 insertions(+)
diff -puN include/asm-s390/types.h~types_h-needs-config_h include/asm-s390/types.h
--- 25/include/asm-s390/types.h~types_h-needs-config_h 2004-03-28 23:02:57.481365480 -0800
+++ 25-akpm/include/asm-s390/types.h 2004-03-28 23:02:57.486364720 -0800
@@ -50,6 +50,8 @@ typedef __signed__ long saddr_t;
*/
#ifdef __KERNEL__
+#include <linux/config.h>
+
#ifndef __s390x__
#define BITS_PER_LONG 32
#else
diff -puN include/asm-sh/types.h~types_h-needs-config_h include/asm-sh/types.h
--- 25/include/asm-sh/types.h~types_h-needs-config_h 2004-03-28 23:02:57.482365328 -0800
+++ 25-akpm/include/asm-sh/types.h 2004-03-28 23:02:57.486364720 -0800
@@ -31,6 +31,8 @@ typedef unsigned long long __u64;
*/
#ifdef __KERNEL__
+#include <linux/config.h>
+
#define BITS_PER_LONG 32
#ifndef __ASSEMBLY__
diff -puN include/asm-x86_64/types.h~types_h-needs-config_h include/asm-x86_64/types.h
--- 25/include/asm-x86_64/types.h~types_h-needs-config_h 2004-03-28 23:02:57.484365024 -0800
+++ 25-akpm/include/asm-x86_64/types.h 2004-03-28 23:02:57.486364720 -0800
@@ -29,6 +29,8 @@ typedef unsigned long long __u64;
*/
#ifdef __KERNEL__
+#include <linux/config.h>
+
#define BITS_PER_LONG 64
#ifndef __ASSEMBLY__
_
Andrew Morton <[email protected]> writes:
> Benjamin Herrenschmidt <[email protected]> wrote:
> >
> > sector_t depends on CONFIG_LBD but include/config.h may not be there
> > thus causing interesting breakage in some places...
>
> Nasty.
>
> > Here's the fix for ppc32 (problem found by Roman Zippel, other archs
> > need a similar fix).
>
> Three of them.
>
> 25-akpm/include/asm-s390/types.h | 2 ++
> 25-akpm/include/asm-sh/types.h | 2 ++
> 25-akpm/include/asm-x86_64/types.h | 2 ++
Please use this change for x86-64 instead.
-Andi
diff -u linux-2.6.5rc2-amd64/include/asm-x86_64/types.h-o linux-2.6.5rc2-amd64/include/asm-x86_64/types.h
--- linux-2.6.5rc2-amd64/include/asm-x86_64/types.h-o 2004-03-21 21:11:54.000000000 +0100
+++ linux-2.6.5rc2-amd64/include/asm-x86_64/types.h 2004-03-29 04:44:24.000000000 +0200
@@ -48,10 +48,8 @@
typedef u64 dma64_addr_t;
typedef u64 dma_addr_t;
-#ifdef CONFIG_LBD
typedef u64 sector_t;
#define HAVE_SECTOR_T
-#endif
#endif /* __ASSEMBLY__ */
Hello all
This one liner patch permits to save 2 instructions in the fget()
function, at least on x86_64 SMP (only one ref to current->files)
diff -Nru linux-2.6.2/fs/file_table.c linux-2.6.2-custom/fs/file_table.c
--- linux-2.6.2/fs/file_table.c 2004-02-04 17:59:43.000000000 +0100
+++ linux-2.6.2-custom/fs/file_table.c 2004-03-29
11:26:42.170555000 +0200
@@ -198,7 +198,7 @@
struct files_struct *files = current->files;
spin_lock(&files->file_lock);
- file = fcheck(fd);
+ file = fcheck_files(files,fd) ;
if (file)
get_file(file);
spin_unlock(&files->file_lock);
The assembly code was (oprofile data included) :
ffffffff8016c3d0 <fget>: /* fget total: 921168 0.1624 */
51191 0.0090 :ffffffff8016c3d0: mov %gs:0x0,%rax
76621 0.0135 :ffffffff8016c3d9: mov 0x7e8(%rax),%rsi
121592 0.0214 :ffffffff8016c3e0: lock decb 0x4(%rsi)
11278 0.0020 :ffffffff8016c3e4: js ffffffff8016c5e8
<.text.lock.file_table+0x20>
122737 0.0216 :ffffffff8016c3ea: mov %gs:0x0,%rax
174870 0.0308 :ffffffff8016c3f3: mov 0x7e8(%rax),%rcx
2274 4.0e-04 :ffffffff8016c3fa: xor %eax,%eax
77418 0.0137 :ffffffff8016c3fc: cmp 0x8(%rcx),%edi
3417 6.0e-04 :ffffffff8016c3ff: jae ffffffff8016c40b <fget+0x3b>
8184 0.0014 :ffffffff8016c401: mov 0x18(%rcx),%rax
37585 0.0066 :ffffffff8016c405: mov %edi,%edx
7357 0.0013 :ffffffff8016c407: mov (%rax,%rdx,8),%rax
5817 0.0010 :ffffffff8016c40b: test %rax,%rax
57910 0.0102 :ffffffff8016c40e: je ffffffff8016c414 <fget+0x44>
3401 6.0e-04 :ffffffff8016c410: lock incl 0x28(%rax)
7729 0.0014 :ffffffff8016c414: movb $0x1,0x4(%rsi)
151787 0.0268 :ffffffff8016c418: retq
And it is now :
380: 65 48 8b 04 25 00 00 mov %gs:0x0,%rax
387: 00 00
389: 48 8b 88 e8 07 00 00 mov 0x7e8(%rax),%rcx
390: f0 fe 49 04 lock decb 0x4(%rcx)
394: 0f 88 ee 01 00 00 js 588 <.text.lock.file_table+0x20>
39a: 31 c0 xor %eax,%eax
39c: 3b 79 08 cmp 0x8(%rcx),%edi
39f: 73 0a jae 3ab <fget+0x2b>
3a1: 48 8b 41 18 mov 0x18(%rcx),%rax
3a5: 89 fa mov %edi,%edx
3a7: 48 8b 04 d0 mov (%rax,%rdx,8),%rax
3ab: 48 85 c0 test %rax,%rax
3ae: 74 04 je 3b4 <fget+0x34>
3b0: f0 ff 40 28 lock incl 0x28(%rax)
3b4: c6 41 04 01 movb $0x1,0x4(%rcx)
3b8: c3 retq
Thanks
On Sun, Mar 28, 2004 at 11:03:51PM -0800, Andrew Morton wrote:
> Benjamin Herrenschmidt <[email protected]> wrote:
> >
> > sector_t depends on CONFIG_LBD but include/config.h may not be there
> > thus causing interesting breakage in some places...
>
> Nasty.
Dont forget trusty old linux/scripts/checkconfig.pl.
Anton
On Sun, Mar 28, 2004 at 11:03:51PM -0800, Andrew Morton wrote:
> > Here's the fix for ppc32 (problem found by Roman Zippel, other archs
> > need a similar fix).
>
> Three of them.
Why do we keep thios in asm-*/types.h instead of linux/types.h, btw?
>
> -#ifdef CONFIG_LBD
> typedef u64 sector_t;
> #define HAVE_SECTOR_T
> -#endif
Do you need that at all then ? The default unsigned long should
be just fine...
Ben.
On Mon, 29 Mar 2004 23:18:23 +1000
Benjamin Herrenschmidt <[email protected]> wrote:
>
> >
> > -#ifdef CONFIG_LBD
> > typedef u64 sector_t;
> > #define HAVE_SECTOR_T
> > -#endif
>
> Do you need that at all then ? The default unsigned long should
> be just fine...
True, it could be completely dropped too.
-Andi
> sector_t depends on CONFIG_LBD but include/config.h may not be there
> thus causing interesting breakage in some places...
> Here's the fix for ppc32 (problem found by Roman Zippel, other archs
> need a similar fix).
This indeed solves the oops when mounting an HFS fs on my ppc32.