Skip to contentSkip to conversation

ppc32: Fix sector_t definition with CONFIG_LBD

PATCH In review Benjamin Herrenschmidt 6 participants 8 messages Mar 29, 2004 1 message not in the archive VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ppc32: Fix sector_t definition with CONFIG_LBD

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).

include/asm-ppc/types.h +2 -0
diff -urN linux-2.5/include/asm-ppc/types.h linuxppc-2.5-benh/include/asm-ppc/types.h
@@ -37,6 +37,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/config.h>
+
 typedef signed char s8;
 typedef unsigned char u8;
 
#1 lore raw
Quoted 3 lines · Benjamin Herrenschmidt

sector_t depends on CONFIG_LBD but include/config.h may not be there thus causing interesting breakage in some places...

Nasty.

Quoted 2 lines

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(+)
include/asm-s390/types.h renamed from include/asm-s390/types.h~types_h-needs-config_h +2 -0
diff -puN include/asm-s390/types.h~types_h-needs-config_h include/asm-s390/types.h
@@ -50,6 +50,8 @@ typedef __signed__ long saddr_t;
  */
 #ifdef __KERNEL__
 
+#include <linux/config.h>
+
 #ifndef __s390x__
 #define BITS_PER_LONG 32
 #else
include/asm-sh/types.h renamed from include/asm-sh/types.h~types_h-needs-config_h +2 -0
diff -puN include/asm-sh/types.h~types_h-needs-config_h include/asm-sh/types.h
@@ -31,6 +31,8 @@ typedef unsigned long long __u64;
  */
 #ifdef __KERNEL__
 
+#include <linux/config.h>
+
 #define BITS_PER_LONG 32
 
 #ifndef __ASSEMBLY__
include/asm-x86_64/types.h renamed from include/asm-x86_64/types.h~types_h-needs-config_h +2 -0
diff -puN include/asm-x86_64/types.h~types_h-needs-config_h include/asm-x86_64/types.h
@@ -29,6 +29,8 @@ typedef unsigned long long  __u64;
  */
 #ifdef __KERNEL__
 
+#include <linux/config.h>
+
 #define BITS_PER_LONG 64
 
 #ifndef __ASSEMBLY__

_

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 7 lines · Andrew Morton
Quoted 3 lines · Benjamin Herrenschmidt

sector_t depends on CONFIG_LBD but include/config.h may not be there thus causing interesting breakage in some places...

Nasty.

Quoted 8 lines
Quoted 2 lines

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

include/asm-x86_64/types.h renamed from include/asm-x86_64/types.h-o +0 -2
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
@@ -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__ */
 
#4 lore raw

PATCH 1/1 small change to fget()

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)

fs/file_table.c 2004-03-29 11:26:42.170555000 +0200 renamed from fs/file_table.c 2004-02-04 17:59:43.000000000 +0100 +1 -1
diff -Nru linux-2.6.2/fs/file_table.c linux-2.6.2-custom/fs/file_table.c
@@ -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

#5 lore raw
In reply to Andi Kleen

include/asm-x86_64/types.h line 54

Quoted 6 lines
include/asm-x86_64/types.h renamed from include/asm-x86_64/types.h-o +0 -2
-#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.

#6 lore raw
Quoted 9 lines · On Mon, 29 Mar 2004 23:18:23 +1000 Benjamin Herrenschmidt…

include/asm-x86_64/types.h line 54

Quoted 6 lines
include/asm-x86_64/types.h renamed from include/asm-x86_64/types.h-o +0 -2
-#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

#7 lore raw
In reply to Andrew Morton
Quoted 6 lines · Andrew Morton
Quoted 3 lines · Benjamin Herrenschmidt

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

#8 lore raw
In reply to Andrew Morton
Quoted 4 lines · Andrew Morton
Quoted 2 lines

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?