lkml.org 
[lkml]   [2008]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3)
* Eran Liberty (liberty@extricom.com) wrote:
> Mathieu Desnoyers wrote:
>> Can you also give us
>>
>> objdump -S --start-address=0xC00BB724 vmlinux | head 20
>>
>> ?
>>
>> Then we could compare the result with the OOPS instruction dump :
>>
>> 7c0802a6 bf61000c 3f60c038 7c3f0b78 90010024 7c7c1b78 7c9d2378 83db32a0
>> 73c00001 7f83e378 7fa4eb78 4082002f <00000000> 2f830000 409e0030 801b32a0
>>
>> Mathieu
>>
>>
>
> to give you more context I have run:
> > powerpc-linux-gnu-objdump -S --start-address=0xC00BB700 vmlinux | head -n
> 60
>
> the discrepancy starts at address:
> c00bb720: 40 82 00 30 <=> 40 82 00 2f
> c00bb724: 4b ff fe 61 <=> 00 00 00 00
>

Ah !

I think I see what could be wrong :

First we have :

static unsigned int ftrace_nop = 0x60000000;

We probably replace the original function call by this nop.

Then we do :

notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
{
static unsigned int op;

/*
* It would be nice to just use create_function_call, but that will
* update the code itself. Here we need to just return the
* instruction that is going to be modified, without modifying the
* code.
*/
addr = GET_ADDR(addr);

/* Set to "bl addr" */
op = 0x48000001 | (ftrace_calc_offset(ip, addr) & 0x03fffffc);

/*
* No locking needed, this must be called via kstop_machine
* which in essence is like running on a uniprocessor machine.
*/
return (unsigned char *)&op;
}

And I guess we must be doing a 0x48000001 | 0x0; or something ?

Also, we have to consider that POWERPC 64 functions are :

/* PowerPC64's functions are data that points to the functions */

And this does not seem to hold for ppc32. Therefore, it is strange to me
that the same code is used for the update.. are we updating the correct
site ?

Mathieu

> vmlinux: file format elf32-powerpc
>
> Disassembly of section .text:
>
> c00bb700 <d_lookup+0x1c>:
> * d_lookup() is protected against the concurrent renames in some unrelated
> * directory using the seqlockt_t rename_lock.
> */
>
> struct dentry * d_lookup(struct dentry * parent, struct qstr * name)
> {
> c00bb700: 7c 3f 0b 78 mr r31,r1
> c00bb704: 90 01 00 24 stw r0,36(r1)
> c00bb708: 7c 7c 1b 78 mr r28,r3
> c00bb70c: 7c 9d 23 78 mr r29,r4
> c00bb710: 83 db 32 a0 lwz r30,12960(r27)
> {
> unsigned ret;
>
> repeat:
> ret = sl->sequence;
> smp_rmb();
> c00bb714: 73 c0 00 01 andi. r0,r30,1
> struct dentry * dentry = NULL;
> unsigned long seq;
>
> do {
> seq = read_seqbegin(&rename_lock);
> dentry = __d_lookup(parent, name);
> c00bb718: 7f 83 e3 78 mr r3,r28
> c00bb71c: 7f a4 eb 78 mr r4,r29
> if (unlikely(ret & 1)) {
> c00bb720: 40 82 00 30 bne- c00bb750 <d_lookup+0x6c>
> c00bb724: 4b ff fe 61 bl c00bb584 <__d_lookup>
> if (dentry)
> c00bb728: 2f 83 00 00 cmpwi cr7,r3,0
> c00bb72c: 40 9e 00 30 bne- cr7,c00bb75c <d_lookup+0x78>
> *
> * If sequence value changed then writer changed data while in section.
> */
> static __always_inline int read_seqretry(const seqlock_t *sl, unsigned
> start)
> {
> smp_rmb();
> c00bb730: 80 1b 32 a0 lwz r0,12960(r27)
> break;
> } while (read_seqretry(&rename_lock, seq));
> c00bb734: 7f 80 f0 00 cmpw cr7,r0,r30
> c00bb738: 41 9e 00 24 beq- cr7,c00bb75c <d_lookup+0x78>
> /* Start of read calculation -- fetch last complete writer token */
> static __always_inline unsigned read_seqbegin(const seqlock_t *sl)
> {
> unsigned ret;
>
> repeat:
> c00bb73c: 7c 1e 03 78 mr r30,r0
> ret = sl->sequence;
> smp_rmb();
> c00bb740: 73 c0 00 01 andi. r0,r30,1
> struct dentry * dentry = NULL;
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68


\
 
 \ /
  Last update: 2008-08-19 16:49    [W:0.100 / U:8.580 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site