Bug#609371: R_SPARC_13

2011-01-18 Thread Richard Mortimer



On 18/01/2011 06:50, David Miller wrote:

From: David Millerda...@davemloft.net
Date: Mon, 17 Jan 2011 16:37:09 -0800 (PST)


So we do end up seeing the R_SPARC_LO10 + R_SPARC_13 sequences in the
final module object.

Therefore, we really should handle R_SPARC_13 in the sparc module loader.


Ok, I now feel like I'm hallucinating.

davem@sunset:~/src/GIT/linux-2.6-stable$ uname -a
Linux sunset 2.6.37 #1 SMP Wed Jan 12 20:14:59 PST 2011 sparc64 GNU/Linux
davem@sunset:~/src/GIT/linux-2.6-stable$ objdump --reloc 
/lib/modules/2.6.37/kernel/net/ipv6/ipv6.ko | grep R_SPARC_13
0c7c R_SPARC_13*ABS*+0x0004
1ae4 R_SPARC_13*ABS*+0x0018
1b0c R_SPARC_13*ABS*+0x0008
  ...
davem@sunset:~/src/GIT/linux-2.6-stable$ lsmod | grep ipv6
ipv6  240422  12
davem@sunset:~/src/GIT/linux-2.6-stable$

I must be missing something obvious.



I think objdump may be distorting the truth a little. I found the 
following in binutils gas/config/tc-sparc.c tc-gen_reloc(). I wonder if 
it is displaying rewritten records rather than displaying the raw 
contents. I haven't traced it through the code but the fact that it is 
obviously working for you means that something like this is going on.


  /* We expand R_SPARC_OLO10 to R_SPARC_LO10 and R_SPARC_13
 on the same location.  */
  if (code == BFD_RELOC_SPARC_OLO10)
{
  relocs[1] = reloc = (arelent *) xmalloc (sizeof (arelent));
  relocs[2] = NULL;

  reloc-sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
  *reloc-sym_ptr_ptr
= symbol_get_bfdsym (section_symbol (absolute_section));
  reloc-address = fixp-fx_frag-fr_address + fixp-fx_where;
  reloc-howto = bfd_reloc_type_lookup (stdoutput, BFD_RELOC_SPARC13);
  reloc-addend = fixp-tc_fix_data;
}

I will try your alignment patch without any R_SPARC_13 related changes 
and see how that goes.


Regards

Richard



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-18 Thread Richard Mortimer
On Tue, 2011-01-18 at 10:52 +, Richard Mortimer wrote:
 
 On 18/01/2011 06:50, David Miller wrote:
  From: David Millerda...@davemloft.net
  Date: Mon, 17 Jan 2011 16:37:09 -0800 (PST)
 
  So we do end up seeing the R_SPARC_LO10 + R_SPARC_13 sequences in the
  final module object.
 
  Therefore, we really should handle R_SPARC_13 in the sparc module loader.
 
  Ok, I now feel like I'm hallucinating.
 
  davem@sunset:~/src/GIT/linux-2.6-stable$ uname -a
  Linux sunset 2.6.37 #1 SMP Wed Jan 12 20:14:59 PST 2011 sparc64 GNU/Linux
  davem@sunset:~/src/GIT/linux-2.6-stable$ objdump --reloc 
  /lib/modules/2.6.37/kernel/net/ipv6/ipv6.ko | grep R_SPARC_13
  0c7c R_SPARC_13*ABS*+0x0004
  1ae4 R_SPARC_13*ABS*+0x0018
  1b0c R_SPARC_13*ABS*+0x0008
...
  davem@sunset:~/src/GIT/linux-2.6-stable$ lsmod | grep ipv6
  ipv6  240422  12
  davem@sunset:~/src/GIT/linux-2.6-stable$
 
  I must be missing something obvious.
 
 
 I think objdump may be distorting the truth a little. I found the 
 following in binutils gas/config/tc-sparc.c tc-gen_reloc(). I wonder if 
 it is displaying rewritten records rather than displaying the raw 
 contents. I haven't traced it through the code but the fact that it is 
 obviously working for you means that something like this is going on.
 
/* We expand R_SPARC_OLO10 to R_SPARC_LO10 and R_SPARC_13
   on the same location.  */
if (code == BFD_RELOC_SPARC_OLO10)
  {
relocs[1] = reloc = (arelent *) xmalloc (sizeof (arelent));
relocs[2] = NULL;
 
reloc-sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
*reloc-sym_ptr_ptr
  = symbol_get_bfdsym (section_symbol (absolute_section));
reloc-address = fixp-fx_frag-fr_address + fixp-fx_where;
reloc-howto = bfd_reloc_type_lookup (stdoutput, BFD_RELOC_SPARC13);
reloc-addend = fixp-tc_fix_data;
  }
 

Dave,

To close this off as a non-issue as far as my boot failures are
concerned I did some further checking and objdump is displaying
R_SPARC_OLO10 as two separate entries. I checked the scsi_mod.ko binary
and found the appropriate Elf64_Rela entry. That has 0x21 as the LSB of
r_info and that is the proper code for R_SPARC_OLO10 which is what you
expected in the first place!
 
objdump displays

1618 R_SPARC_LO10  __tracepoint_module_get
1618 R_SPARC_13*ABS*+0x0008

The binary contains

0505140 00 00 00 00 00 00 16 18 00 00 04 0b 00 00 08 21
 ^^
0505160 00 00 00 00 00 00 00 00

which corresponds to

typedef struct elf64_rela {
  Elf64_Addr r_offset;  /* Location at which to apply the action */
  Elf64_Xword r_info;   /* index and type of relocation */
  Elf64_Sxword r_addend;/* Constant addend used to compute value */
} Elf64_Rela;

Regards

Richard

 I will try your alignment patch without any R_SPARC_13 related changes 
 and see how that goes.
 
 Regards
 
 Richard
 --
 To unsubscribe from this list: send the line unsubscribe sparclinux in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html





-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-18 Thread David Miller
From: Richard Mortimer ri...@oldelvet.org.uk
Date: Tue, 18 Jan 2011 13:23:14 +

 To close this off as a non-issue as far as my boot failures are
 concerned I did some further checking and objdump is displaying
 R_SPARC_OLO10 as two separate entries. I checked the scsi_mod.ko binary
 and found the appropriate Elf64_Rela entry. That has 0x21 as the LSB of
 r_info and that is the proper code for R_SPARC_OLO10 which is what you
 expected in the first place!

Thanks for figuring this out Richard.

I'll look into fixing binutils so that it properly reports the
correct R_SPARC_OLO10 relocation in dumps.  There really is no
excuse for what it's currently doing.  In fact, I think this
quirk has sent me on wild goose chases in the past.




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-18 Thread David Miller
From: David Miller da...@davemloft.net
Date: Tue, 18 Jan 2011 13:00:27 -0800 (PST)

 I'll look into fixing binutils so that it properly reports the
 correct R_SPARC_OLO10 relocation in dumps.  There really is no
 excuse for what it's currently doing.  In fact, I think this
 quirk has sent me on wild goose chases in the past.

Ok, after looking into it I got reminded why this is still behaving
the way that it is.

The problem is that R_SPARC_OLO10 relocations have 2 addends instead
of one.  But the BFD libraries generic arelent structure only has
room to store one of the addends.

So the 64-bit ELF Sparc backend emits R_SPARC_OLO10 as R_SPARC_LO10 +
R_SPARC_13 internally solely for the purpose of being able to store
the second addend away somewhere.

This all happens in the relocation table slurping code in
bfd/elf64-sparc.c, and this is what objdump uses to get it's info.
BTW, readelf prints the relocations of this type properly and
without this weird translation.

I considered several avenues to make this translation scheme
unnecessary.

Making arelent larger in order to store the second addend would be
frowned upon since only 64-bit sparc needs it but the space cost would
be eaten by everyone.

The other idea was to somehow make the existing addend field smaller
on 64-bit so we could add the second addend storage to arelent at
zero cost.  But after studying the binutils code I'm pretty sure
that the addend field really does need to be a full 64-bits.

This really needs to be fixed for another reason, which is that this
scheme requires allocating twice as much memory to store relocations
internally.  This is because we have to size the internal relocation
table in BFD before we scan the table and know if any R_SPARC_OLO10
compound relocs actually exist and if so how many.

An even worse case is MIPS 64-bit, where every relocation expands to
3 (?!?!) internal BFD relocation objects.

Anyways just a detailed brain dump on what the situation is with this,
I'll see if I can come up with other ideas.




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13 (Re: Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36)

2011-01-17 Thread Richard Mortimer
On Mon, 2011-01-17 at 10:22 +, Richard Mortimer wrote:
 On Sun, 2011-01-16 at 22:07 -0800, David Miller wrote:
  From: David Miller da...@davemloft.net
  Date: Sat, 15 Jan 2011 21:17:22 -0800 (PST)
 
   I think the problem we have here is that the _ftrace_events section is
   not aligned sufficiently.  That .align 4 mnemonic is a good indication
   of this.  It should at least 8 on sparc64.
  
 I noticed another potentially 64 bit unfriendly alignment on struct
 tracepoint in include/linux/tracepoint.h. I don't think that the
 alignment of 32 breaks anything but it does leave a 24 byte hole. I
 don't know enough about tracing to know if that is necessary.
 
 struct tracepoint {
 const char *name;   /* Tracepoint name */
 int state;  /* State. */
 void (*regfunc)(void);
 void (*unregfunc)(void);
 struct tracepoint_func *funcs;
 }  __attribute__((aligned(32)));/*
  * Aligned on 32 bytes because it is
  * globally visible and gcc happily
  * align these on the structure size.
  * Keep in sync with vmlinux.lds.h.
  */
 
 Note I spotted this when looking at some residual sparc64 relocation
 issues when _ftrace_events alignment is changed to 8. I'll follow those
 issues up in a separate email when I get time later today.

I did some experiments looking at what happens when the
include/trace/ftrace.h __aligned__(4) usages for _ftrace_events are
changed to __aligned__(8). This causes the R_SPARC_UA64 relocations to
go to R_SPARC_64 and gets rid of that particular issue.

However it does not cause the R_SPARC_13 relocation records to go away.
They still persist and the ones I've looked at are be related to uses of
struct tracepoint that I mentioned earlier. I tried various different
values of alignment for both __ftrace_events and struct tracepoint but
nothing made the uses of R_SPARC_13 go away.

As an example from drivers/scsi/scsi_error.c function scsi_eh_wakeup().

This has relocation records of

2be0 R_SPARC_HI22  __tracepoint_scsi_eh_wakeup
2be4 R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
2be4 R_SPARC_13*ABS*+0x0008
2bf4 R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
2bf4 R_SPARC_13*ABS*+0x0020

for the following assembly code

sethi  %hi(__tracepoint_scsi_eh_wakeup), %g1   !, tmp135
lduw[%g1+%lo(__tracepoint_scsi_eh_wakeup)+8], %g2   ! 
__tracepoint_scsi_eh_wakeup.state,
cmp %g2, 0  ! __tracepoint_scsi_eh_wakeup.state,
be,pt   %icc, .LL454
nop!
ldx [%g1+%lo(__tracepoint_scsi_eh_wakeup)+32], %l0  !, it_func_ptr
brz,pn  %l0, .LL454 ! it_func_ptr,

this corresponds to the first few lines of

void scsi_eh_wakeup(struct Scsi_Host *shost)
{
if (shost-host_busy == shost-host_failed) {
trace_scsi_eh_wakeup(shost);
wake_up_process(shost-ehandler);
SCSI_LOG_ERROR_RECOVERY(5,
printk(Waking error handler thread\n));
}
}


If I try compiling with the -Os option removed from KBUILD_CFLAGS it
produces a more traditional R_SPARC_HI22 and R_SPARC_LO10 output as
shown below. So maybe there is a really need to add R_SPARC_13 support
to the sparc64 module loader to allow for the optimizations that the
compiler is making now.

01bc R_SPARC_HI22  __tracepoint_scsi_eh_wakeup
01c0 R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
01dc R_SPARC_HI22  
__tracepoint_scsi_eh_wakeup+0x0020
01e0 R_SPARC_LO10  
__tracepoint_scsi_eh_wakeup+0x0020


ldx [%fp+2175], %g1 ! shost, tmp123
stx %g1, [%fp+2007] ! tmp123, shost
sethi   %hi(__tracepoint_scsi_eh_wakeup), %g1   !, tmp125
or  %g1, %lo(__tracepoint_scsi_eh_wakeup), %g1  ! tmp125,, 
tmp124
ld  [%g1+8], %g1! __tracepoint_scsi_eh_wakeup.state, D.32824
xor %g1, 0, %g1 ! D.32824,, tmp126
subcc   %g0, %g1, %g0   !, tmp126
addx%g0, 0, %g1 !,, D.32823
brz %g1, .LL3
 nop! D.32822,
sethi   %hi(__tracepoint_scsi_eh_wakeup+32), %g1!, tmp127
or  %g1, %lo(__tracepoint_scsi_eh_wakeup+32), %g1   ! tmp127,, 
D.32820
ldx [%g1], %g1  !* D.32820, tmp128
stx %g1, [%fp+2015] ! tmp128, _p1

Regards

Richard




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread David Miller
From: Richard Mortimer ri...@oldelvet.org.uk
Date: Mon, 17 Jan 2011 19:46:21 +

 As an example from drivers/scsi/scsi_error.c function scsi_eh_wakeup().
 
 This has relocation records of
 ...
 2be4 R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
 2be4 R_SPARC_13*ABS*+0x0008
 ...
 lduw[%g1+%lo(__tracepoint_scsi_eh_wakeup)+8], %g2   ! 
 __tracepoint_scsi_eh_wakeup.state,

In a final object, the binutils linker should be using one
R_SPARC_OLO10 relocation for this kind of expression on sparc64.  Not
the two relocations on the same instruction it appears to be using
here.  I think you're looking at an object output by the assembler
and not the finally linked module.

You should also be careful about which objects you are analyzing.  You
should be looking at the finally linked foo.ko file, not the
individual foo.o objects, as the majority of the relocations go away
when the linker puts together the final module.

Is that what you're doing?



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread Richard Mortimer
On Mon, 2011-01-17 at 13:02 -0800, David Miller wrote:
 From: Richard Mortimer ri...@oldelvet.org.uk
 Date: Mon, 17 Jan 2011 19:46:21 +
 
  As an example from drivers/scsi/scsi_error.c function scsi_eh_wakeup().
  
  This has relocation records of
  ...
  2be4 R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
  2be4 R_SPARC_13*ABS*+0x0008
  ...
  lduw[%g1+%lo(__tracepoint_scsi_eh_wakeup)+8], %g2   ! 
  __tracepoint_scsi_eh_wakeup.state,
 
 In a final object, the binutils linker should be using one
 R_SPARC_OLO10 relocation for this kind of expression on sparc64.  Not
 the two relocations on the same instruction it appears to be using
 here.  I think you're looking at an object output by the assembler
 and not the finally linked module.
 
 You should also be careful about which objects you are analyzing.  You
 should be looking at the finally linked foo.ko file, not the
 individual foo.o objects, as the majority of the relocations go away
 when the linker puts together the final module.
 
 Is that what you're doing?

Yes in this instance I was/am. Thanks for the explanation.

However the same R_SPARC_13 also exists in scsi_mod.ko. It exists in the
original Debian 2.6.37-trunk-sparc64 version and in my current build of
the same with the 8 byte alignment for _trace_events.


74a8 R_SPARC_HI22  __tracepoint_scsi_eh_wakeup
74ac R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
74ac R_SPARC_13*ABS*+0x0008
74bc R_SPARC_LO10  __tracepoint_scsi_eh_wakeup
74bc R_SPARC_13*ABS*+0x0020

74a8:   03 00 00 00 sethi  %hi(0), %g1
74ac:   c4 00 60 00 ld  [ %g1 ], %g2
74b0:   80 a0 a0 00 cmp  %g2, 0
74b4:   02 48 00 0c be  %icc, 74e4 scsi_eh_wakeup+0x50
74b8:   01 00 00 00 nop
74bc:   e0 58 60 00 ldx  [ %g1 ], %l0

I guess that points towards the binutils linker not doing the correct
thing.

ld reports its version as

$ ld -v
GNU ld (GNU Binutils for Debian) 2.20.1-system.20100303

and scsi_mod.ko is linked with the following command

ld -r -m elf64_sparc -T /richmtmp/linux-2.6-2.6.37/scripts/module-common.lds 
--build-id  -o drivers/scsi/scsi_mod.ko drivers/scsi/scsi_mod.o 
drivers/scsi/scsi_mod.mod.o

Regards

Richard




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread David Miller
From: Richard Mortimer ri...@oldelvet.org.uk
Date: Mon, 17 Jan 2011 23:34:03 +

 However the same R_SPARC_13 also exists in scsi_mod.ko. It exists in the
 original Debian 2.6.37-trunk-sparc64 version and in my current build of
 the same with the 8 byte alignment for _trace_events.
 ...

Thanks for the info Richard, I'll look more deeply into this.



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread David Miller
From: Richard Mortimer ri...@oldelvet.org.uk
Date: Mon, 17 Jan 2011 23:34:03 +

 I guess that points towards the binutils linker not doing the correct
 thing.

Ok, it is in fact doing the correct thing.

I'm really surprised we never hit this before in all of these years
:-) I guess we've simply never hit this kind of expression in a module
before.

The issue is that modules aren't a final link, it's really more like
an intermediate partial link.

So we do end up seeing the R_SPARC_LO10 + R_SPARC_13 sequences in the
final module object.

Therefore, we really should handle R_SPARC_13 in the sparc module loader.

Richard, I want you to get full credit for this since you did all of
the dirty work :-) Would you please cons up a formal patch with commit
message and signoff for this and I'll push it around?

Thanks a lot!



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread Richard Mortimer



On 18/01/2011 00:37, David Miller wrote:

From: Richard Mortimerri...@oldelvet.org.uk
Date: Mon, 17 Jan 2011 23:34:03 +


I guess that points towards the binutils linker not doing the correct
thing.


Ok, it is in fact doing the correct thing.

I'm really surprised we never hit this before in all of these years
:-) I guess we've simply never hit this kind of expression in a module
before.

The issue is that modules aren't a final link, it's really more like
an intermediate partial link.

So we do end up seeing the R_SPARC_LO10 + R_SPARC_13 sequences in the
final module object.

Therefore, we really should handle R_SPARC_13 in the sparc module loader.

Richard, I want you to get full credit for this since you did all of
the dirty work :-) Would you please cons up a formal patch with commit
message and signoff for this and I'll push it around?

Thanks a lot!


Will do tomorrow. I'll dust off my git tree.

Regards

Richard



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#609371: R_SPARC_13

2011-01-17 Thread David Miller
From: David Miller da...@davemloft.net
Date: Mon, 17 Jan 2011 16:37:09 -0800 (PST)

 So we do end up seeing the R_SPARC_LO10 + R_SPARC_13 sequences in the
 final module object.
 
 Therefore, we really should handle R_SPARC_13 in the sparc module loader.

Ok, I now feel like I'm hallucinating.

davem@sunset:~/src/GIT/linux-2.6-stable$ uname -a
Linux sunset 2.6.37 #1 SMP Wed Jan 12 20:14:59 PST 2011 sparc64 GNU/Linux
davem@sunset:~/src/GIT/linux-2.6-stable$ objdump --reloc 
/lib/modules/2.6.37/kernel/net/ipv6/ipv6.ko | grep R_SPARC_13
0c7c R_SPARC_13*ABS*+0x0004
1ae4 R_SPARC_13*ABS*+0x0018
1b0c R_SPARC_13*ABS*+0x0008
 ...
davem@sunset:~/src/GIT/linux-2.6-stable$ lsmod | grep ipv6
ipv6  240422  12 
davem@sunset:~/src/GIT/linux-2.6-stable$ 

I must be missing something obvious.




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org