Hi!
This introduces meaningfull labels instead of .L1234, meaning code is
readable, kills alignment where unneccessary, and kills TLB flush that
was only pure paranoia (and slows it down a lot on emulated
systems). Please apply,
Pavel
--- linux-cvs//arch/i386/power/swsusp.S 2004-05-25 17:41:18.000000000 +0200
+++ linux/arch/i386/power/swsusp.S 2004-06-24 14:39:01.000000000 +0200
@@ -18,7 +18,7 @@
ENTRY(do_magic)
pushl %ebx
cmpl $0,8(%esp)
- jne .L1450
+ jne resume
call do_magic_suspend_1
call save_processor_state
@@ -33,21 +33,21 @@
pushfl ; popl saved_context_eflags
call do_magic_suspend_2
- jmp .L1449
- .p2align 4,,7
-.L1450:
+ popl %ebx
+ ret
+
+resume:
movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
movl %ecx,%cr3
call do_magic_resume_1
movl $0,loop
cmpl $0,nr_copy_pages
- je .L1453
- .p2align 4,,7
-.L1455:
+ je copy_done
+copy_loop:
movl $0,loop2
.p2align 4,,7
-.L1459:
+copy_one_page:
movl pagedir_nosave,%ecx
movl loop,%eax
movl loop2,%edx
@@ -56,23 +56,21 @@
movl (%ecx,%eax),%eax
movb (%edx,%eax),%al
movb %al,(%edx,%ebx)
- movl %cr3, %eax;
- movl %eax, %cr3; # flush TLB
movl loop2,%eax
leal 1(%eax),%edx
movl %edx,loop2
movl %edx,%eax
cmpl $4095,%eax
- jbe .L1459
+ jbe copy_one_page
movl loop,%eax
leal 1(%eax),%edx
movl %edx,loop
movl %edx,%eax
cmpl nr_copy_pages,%eax
- jb .L1455
- .p2align 4,,7
-.L1453:
+ jb copy_loop
+
+copy_done:
movl $__USER_DS,%eax
movw %ax, %ds
@@ -88,7 +86,6 @@
call restore_processor_state
pushl saved_context_eflags ; popfl
call do_magic_resume_2
-.L1449:
popl %ebx
ret
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
On Fri, 25 Jun 2004, Pavel Machek wrote:
> Hi!
>
> This introduces meaningfull labels instead of .L1234, meaning code is
> readable, kills alignment where unneccessary, and kills TLB flush that
> was only pure paranoia (and slows it down a lot on emulated
> systems). Please apply,
>
> Pavel
>
> --- linux-cvs//arch/i386/power/swsusp.S 2004-05-25 17:41:18.000000000 +0200
> +++ linux/arch/i386/power/swsusp.S 2004-06-24 14:39:01.000000000 +0200
> @@ -18,7 +18,7 @@
> ENTRY(do_magic)
> pushl %ebx
> cmpl $0,8(%esp)
> - jne .L1450
> + jne resume
> call do_magic_suspend_1
> call save_processor_state
>
> @@ -33,21 +33,21 @@
> pushfl ; popl saved_context_eflags
>
> call do_magic_suspend_2
> - jmp .L1449
> - .p2align 4,,7
> -.L1450:
> + popl %ebx
> + ret
> +
> +resume:
> movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
> movl %ecx,%cr3
>
> call do_magic_resume_1
> movl $0,loop
> cmpl $0,nr_copy_pages
> - je .L1453
> - .p2align 4,,7
> -.L1455:
> + je copy_done
> +copy_loop:
> movl $0,loop2
> .p2align 4,,7
> -.L1459:
> +copy_one_page:
> movl pagedir_nosave,%ecx
> movl loop,%eax
> movl loop2,%edx
> @@ -56,23 +56,21 @@
> movl (%ecx,%eax),%eax
> movb (%edx,%eax),%al
> movb %al,(%edx,%ebx)
> - movl %cr3, %eax;
> - movl %eax, %cr3; # flush TLB
>
> movl loop2,%eax
> leal 1(%eax),%edx
> movl %edx,loop2
> movl %edx,%eax
> cmpl $4095,%eax
> - jbe .L1459
> + jbe copy_one_page
> movl loop,%eax
> leal 1(%eax),%edx
> movl %edx,loop
> movl %edx,%eax
> cmpl nr_copy_pages,%eax
> - jb .L1455
> - .p2align 4,,7
> -.L1453:
> + jb copy_loop
> +
> +copy_done:
> movl $__USER_DS,%eax
>
> movw %ax, %ds
> @@ -88,7 +86,6 @@
> call restore_processor_state
> pushl saved_context_eflags ; popfl
> call do_magic_resume_2
> -.L1449:
> popl %ebx
> ret
>
>
> --
NO! You just made those labels public! The LOCAL symbols need to
begin with ".L". Now, if you have a 'copy_loop' in another module,
linked with this, anywhere in the kernel, they will share the
same address -- not what you expected, I'm sure! The assembler
has some strange rules you need to understand. Use `nm` and you
will find that your new labels are in the object file!
Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.
On Fri, Jun 25, 2004 at 08:37:45AM -0400, Richard B. Johnson wrote:
> NO! You just made those labels public! The LOCAL symbols need to
> begin with ".L". Now, if you have a 'copy_loop' in another module,
> linked with this, anywhere in the kernel, they will share the
> same address -- not what you expected, I'm sure! The assembler
> has some strange rules you need to understand. Use `nm` and you
> will find that your new labels are in the object file!
Er, no. They'll show up in the object file. That doesn't mean they're
global; static symbols also show up in the object file.
--
Daniel Jacobowitz
On Fri, 25 Jun 2004, Daniel Jacobowitz wrote:
> On Fri, Jun 25, 2004 at 08:37:45AM -0400, Richard B. Johnson wrote:
> > NO! You just made those labels public! The LOCAL symbols need to
> > begin with ".L". Now, if you have a 'copy_loop' in another module,
> > linked with this, anywhere in the kernel, they will share the
> > same address -- not what you expected, I'm sure! The assembler
> > has some strange rules you need to understand. Use `nm` and you
> > will find that your new labels are in the object file!
>
> Er, no. They'll show up in the object file. That doesn't mean they're
> global; static symbols also show up in the object file.
>
> --
> Daniel Jacobowitz
I got caught on these, thinking that they weren't globals when
I made a assembly files that used, not only named labels but
also definitions like:
BUF = 0x08
LEN = 0x0c
code: movl BUF(%esp), %ebx
movl LEN(%esp), %ecx
This was done in several files. When linked, I got 'duplicate synbol'
errors. These symbols, while not 'globals' in the sense that 'C'
code can link with them are global definitions that are seen by
the linker and cause duplicate symbol errors. I went through
the gas documentation, trying to find how to prevent these
private symbols from being written to the object file and there
is no command-line mechanism. You just have to start every name
with .L to keep them local.
When I used labels like 'code' above, even though not declared
'.global', the labels also showed up as duplicate symbols when
linking the resulting object files.
Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.
Hi!
> > This introduces meaningfull labels instead of .L1234, meaning code is
> > readable, kills alignment where unneccessary, and kills TLB flush that
> > was only pure paranoia (and slows it down a lot on emulated
> > systems). Please apply,
> >
> > Pavel
> >
> > --- linux-cvs//arch/i386/power/swsusp.S 2004-05-25 17:41:18.000000000 +0200
> > +++ linux/arch/i386/power/swsusp.S 2004-06-24 14:39:01.000000000 +0200
> > @@ -18,7 +18,7 @@
> > ENTRY(do_magic)
> > pushl %ebx
> > cmpl $0,8(%esp)
> > - jne .L1450
> > + jne resume
> > call do_magic_suspend_1
> > call save_processor_state
> >
...
> NO! You just made those labels public! The LOCAL symbols need to
> begin with ".L". Now, if you have a 'copy_loop' in another module,
> linked with this, anywhere in the kernel, they will share the
> same address -- not what you expected, I'm sure! The assembler
> has some strange rules you need to understand. Use `nm` and you
> will find that your new labels are in the object file!
Are you sure? I thought theare not visible from other moduless unless
"ENTRY()" is used. See for example entry.S.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
On Fri, 25 Jun 2004, Pavel Machek wrote:
> Hi!
>
> > > This introduces meaningfull labels instead of .L1234, meaning code is
> > > readable, kills alignment where unneccessary, and kills TLB flush that
> > > was only pure paranoia (and slows it down a lot on emulated
> > > systems). Please apply,
> > >
> > > Pavel
> > >
> > > --- linux-cvs//arch/i386/power/swsusp.S 2004-05-25 17:41:18.000000000 +0200
> > > +++ linux/arch/i386/power/swsusp.S 2004-06-24 14:39:01.000000000 +0200
> > > @@ -18,7 +18,7 @@
> > > ENTRY(do_magic)
> > > pushl %ebx
> > > cmpl $0,8(%esp)
> > > - jne .L1450
> > > + jne resume
> > > call do_magic_suspend_1
> > > call save_processor_state
> > >
> ...
> > NO! You just made those labels public! The LOCAL symbols need to
> > begin with ".L". Now, if you have a 'copy_loop' in another module,
> > linked with this, anywhere in the kernel, they will share the
> > same address -- not what you expected, I'm sure! The assembler
> > has some strange rules you need to understand. Use `nm` and you
> > will find that your new labels are in the object file!
>
> Are you sure? I thought theare not visible from other moduless unless
> "ENTRY()" is used. See for example entry.S.
> Pavel
Well if you find a way to make those symbols disappear from the
object files without using ".L", please let me know. I spent
some considerable time beautifying multiple assembly-language
files on a recent project, only to find that it would fail to
link because of the "multiple symbol definition" errors. I
had to change everything back.
You can make labels like ".Lresume:" remain visible only in
the file. They don't need to be numbers. It may even be possible
to use labels like ".L_resume:", I haven't tried. Basically, if
they show up using `nm`, they can (read will) give you problems.
Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.
On Fri, Jun 25, 2004 at 12:02:47PM -0400, Richard B. Johnson wrote:
> On Fri, 25 Jun 2004, Daniel Jacobowitz wrote:
>
> > On Fri, Jun 25, 2004 at 08:37:45AM -0400, Richard B. Johnson wrote:
> > > NO! You just made those labels public! The LOCAL symbols need to
> > > begin with ".L". Now, if you have a 'copy_loop' in another module,
> > > linked with this, anywhere in the kernel, they will share the
> > > same address -- not what you expected, I'm sure! The assembler
> > > has some strange rules you need to understand. Use `nm` and you
> > > will find that your new labels are in the object file!
> >
> > Er, no. They'll show up in the object file. That doesn't mean they're
> > global; static symbols also show up in the object file.
> >
> > --
> > Daniel Jacobowitz
>
> I got caught on these, thinking that they weren't globals when
> I made a assembly files that used, not only named labels but
> also definitions like:
>
> BUF = 0x08
> LEN = 0x0c
>
> code: movl BUF(%esp), %ebx
> movl LEN(%esp), %ecx
>
>
> This was done in several files. When linked, I got 'duplicate synbol'
> errors. These symbols, while not 'globals' in the sense that 'C'
> code can link with them are global definitions that are seen by
> the linker and cause duplicate symbol errors. I went through
Those are what gas calls absolute symbols.
> the gas documentation, trying to find how to prevent these
> private symbols from being written to the object file and there
> is no command-line mechanism. You just have to start every name
> with .L to keep them local.
>
> When I used labels like 'code' above, even though not declared
> '.global', the labels also showed up as duplicate symbols when
> linking the resulting object files.
Does this only happen in your world? Or were you using a broken
non-GNU linker?
drow@nevyn:~% cat a.s
BUF = 0x1
code:
mov %eax, 0
drow@nevyn:~% as -o a.o a.s
drow@nevyn:~% as -o b.o a.s
drow@nevyn:~% ld -o a a.o b.o
ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074
drow@nevyn:~% nm a.o
00000001 a BUF
00000000 t code
drow@nevyn:~% nm a
00000001 a BUF
00000001 a BUF
08049084 A __bss_start
08049084 A _edata
08049084 A _end
08048074 t code
0804807c t code
LD won't even warn about duplicate absolute symbols unless they have
different values.
--
Daniel Jacobowitz
On Fri, 25 Jun 2004, Daniel Jacobowitz wrote:
> On Fri, Jun 25, 2004 at 12:02:47PM -0400, Richard B. Johnson wrote:
> > On Fri, 25 Jun 2004, Daniel Jacobowitz wrote:
> >
> > > On Fri, Jun 25, 2004 at 08:37:45AM -0400, Richard B. Johnson wrote:
> > > > NO! You just made those labels public! The LOCAL symbols need to
> > > > begin with ".L". Now, if you have a 'copy_loop' in another module,
> > > > linked with this, anywhere in the kernel, they will share the
> > > > same address -- not what you expected, I'm sure! The assembler
> > > > has some strange rules you need to understand. Use `nm` and you
> > > > will find that your new labels are in the object file!
> > >
> > > Er, no. They'll show up in the object file. That doesn't mean they're
> > > global; static symbols also show up in the object file.
> > >
> > > --
> > > Daniel Jacobowitz
> >
> > I got caught on these, thinking that they weren't globals when
> > I made a assembly files that used, not only named labels but
> > also definitions like:
> >
> > BUF = 0x08
> > LEN = 0x0c
> >
> > code: movl BUF(%esp), %ebx
> > movl LEN(%esp), %ecx
> >
> >
> > This was done in several files. When linked, I got 'duplicate synbol'
> > errors. These symbols, while not 'globals' in the sense that 'C'
> > code can link with them are global definitions that are seen by
> > the linker and cause duplicate symbol errors. I went through
>
> Those are what gas calls absolute symbols.
>
> > the gas documentation, trying to find how to prevent these
> > private symbols from being written to the object file and there
> > is no command-line mechanism. You just have to start every name
> > with .L to keep them local.
> >
> > When I used labels like 'code' above, even though not declared
> > '.global', the labels also showed up as duplicate symbols when
> > linking the resulting object files.
>
> Does this only happen in your world? Or were you using a broken
> non-GNU linker?
>
It is the GNU linker
> drow@nevyn:~% cat a.s
> BUF = 0x1
> code:
> mov %eax, 0
> drow@nevyn:~% as -o a.o a.s
> drow@nevyn:~% as -o b.o a.s
> drow@nevyn:~% ld -o a a.o b.o
> ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074
> drow@nevyn:~% nm a.o
> 00000001 a BUF
> 00000000 t code
> drow@nevyn:~% nm a
> 00000001 a BUF
> 00000001 a BUF
> 08049084 A __bss_start
> 08049084 A _edata
> 08049084 A _end
> 08048074 t code
> 0804807c t code
>
> LD won't even warn about duplicate absolute symbols unless they have
> different values.
>
But they ARE different values. They will probably always be different
values, and they end up with the same name, that `ld` sees and complains.
> --
> Daniel Jacobowitz
>
Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.
> > drow@nevyn:~% cat a.s
> > BUF = 0x1
> > code:
> > mov %eax, 0
> > drow@nevyn:~% as -o a.o a.s
> > drow@nevyn:~% as -o b.o a.s
> > drow@nevyn:~% ld -o a a.o b.o
> > ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074
> > drow@nevyn:~% nm a.o
> > 00000001 a BUF
> > 00000000 t code
> > drow@nevyn:~% nm a
> > 00000001 a BUF
> > 00000001 a BUF
> > 08049084 A __bss_start
> > 08049084 A _edata
> > 08049084 A _end
> > 08048074 t code
> > 0804807c t code
> >
> > LD won't even warn about duplicate absolute symbols unless they have
> > different values.
> >
>
> But they ARE different values. They will probably always be different
> values, and they end up with the same name, that `ld` sees and complains.
No, it doesn't complain, that's the point. According to nm(1):
For each symbol, nm shows:
The symbol value, in the radix selected by options
(see below), or hexadecimal by default.
The symbol type. At least the following types are
used; others are, as well, depending on the object
file format. If lowercase, the symbol is local; if
uppercase, the symbol is global (external).
Symbols in a source file that are static are tagged with a 't' as well.
Read System.map for more proof of this.
The patch is fine, especially since I think it's based on something I've
posted in the past. :) The one thing I'd always done differently, though,
was to prefix the labels with '.', since I thought that was what kept it
local. But, after I RTFM'd, I find that's not even necessary.
Pat