2002-02-11 04:48:17

by John Weber

[permalink] [raw]
Subject: 2.5.4 Compile Error

Will try to specify the problem more precisely after I play around for a
bit, but here goes the error:

[root@boolean linux-2.5.4]# make bzImage
gcc -D__KERNEL__ -I/usr/src/linux-2.5.4/include -Wall
-Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer
-fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2
-march=i686 -DKBUILD_BASENAME=main -c -o init/main.o init/main.c
In file included from /usr/src/linux-2.5.4/include/asm/thread_info.h:13,
from /usr/src/linux-2.5.4/include/linux/thread_info.h:10,
from /usr/src/linux-2.5.4/include/linux/spinlock.h:7,
from /usr/src/linux-2.5.4/include/linux/mmzone.h:8,
from /usr/src/linux-2.5.4/include/linux/gfp.h:4,
from /usr/src/linux-2.5.4/include/linux/slab.h:14,
from /usr/src/linux-2.5.4/include/linux/proc_fs.h:5,
from init/main.c:15:
/usr/src/linux-2.5.4/include/asm/processor.h: In function `thread_saved_pc':
/usr/src/linux-2.5.4/include/asm/processor.h:444: dereferencing pointer
to incomplete type
make: *** [init/main.o] Error 1


2002-02-11 05:04:41

by Jeff Garzik

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

John Weber wrote:
> /usr/src/linux-2.5.4/include/asm/processor.h: In function `thread_saved_pc':
> /usr/src/linux-2.5.4/include/asm/processor.h:444: dereferencing pointer
> to incomplete type
> make: *** [init/main.o] Error 1

since it's just for /usr/bin/ps, ie. not a fast path, I just un-inlined
it in my alpha hacking. Same approach might work for here, too.

The basic problem, I'm guessing, is that asm/processor.h wants to know
about the internals of task struct, but it can't yet.

Jeff




--
Jeff Garzik | "I went through my candy like hot oatmeal
Building 1024 | through an internally-buttered weasel."
MandrakeSoft | - goats.com

2002-02-11 06:02:52

by John Weber

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

Jeff Garzik wrote:
> John Weber wrote:
>
>>/usr/src/linux-2.5.4/include/asm/processor.h: In function `thread_saved_pc':
>>/usr/src/linux-2.5.4/include/asm/processor.h:444: dereferencing pointer
>>to incomplete type
>>make: *** [init/main.o] Error 1
>>
>
> since it's just for /usr/bin/ps, ie. not a fast path, I just un-inlined
> it in my alpha hacking. Same approach might work for here, too.
>
> The basic problem, I'm guessing, is that asm/processor.h wants to know
> about the internals of task struct, but it can't yet.
>
> Jeff
>

I don't know what the problem is, but un-inlining this function isn't
correcting it.

The function thread_saved_pc() is a mystery to me. It is declared with
a return type of unsigned long, and yet return this:

((unsigned long *)tsk->thread->esp)[3]

This is confusing to me in many ways:
- the "thread" member of task struct is not a pointer
- esp is of type unsigned long, so I don't understand the cast, and
I certainly don't understand the [3] here.

Can anyone explain this code to me?

I'm a kernelnewbie, so I'm inclined to return:
return (tsk->thread).esp
What is this function trying to do?

2002-02-11 06:21:05

by Robert Love

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

On Mon, 2002-02-11 at 01:02, John Weber wrote:

> The function thread_saved_pc() is a mystery to me. It is declared with
> a return type of unsigned long, and yet return this:
>
> ((unsigned long *)tsk->thread->esp)[3]
>
> This is confusing to me in many ways:
> - the "thread" member of task struct is not a pointer
> - esp is of type unsigned long, so I don't understand the cast, and
> I certainly don't understand the [3] here.
>
> Can anyone explain this code to me?

The problem is an interdependency between processor.h and sched.h.

The old code was the same, except it did

t->esp

where t was a thread_struct, instead of what we do now

t->thread->esp

where t is a task_struct. And thus whereby before we passed

p->thread

as the argument, now you pass just `p'. I.e., its the same net-affect.
The error is because the function needs access to both task_struct (in
sched.h) and thread_struct (in processor.h) but the two are interrelated
so we can't include them in each other.

The contents of esp is a memory address, so typecasting it to (unsigned
long *) is OK.

As for the [3], p[3] is the same as
*(p+3)
ie,
*(p+sizeof(p))
so that is legal.

So the fix, aside from reverting this change and the kernel/sched.c
change and the sparc64 changes ... would be to solve the dependency
issue.

Robert Love

2002-02-11 06:31:31

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

John Weber wrote:
>
> I don't know what the problem is, but un-inlining this function isn't
> correcting it.
>

Try this:

--- linux-2.5.4/include/asm-i386/processor.h Sun Feb 10 22:00:29 2002
+++ 25/include/asm-i386/processor.h Sun Feb 10 22:21:53 2002
@@ -435,14 +435,7 @@ extern int kernel_thread(int (*fn)(void
/* Copy and release all segment info associated with a VM */
extern void copy_segments(struct task_struct *p, struct mm_struct * mm);
extern void release_segments(struct mm_struct * mm);
-
-/*
- * Return saved PC of a blocked thread.
- */
-static inline unsigned long thread_saved_pc(struct task_struct *tsk)
-{
- return ((unsigned long *)tsk->thread->esp)[3];
-}
+extern unsigned long thread_saved_pc(struct task_struct *tsk);

unsigned long get_wchan(struct task_struct *p);
#define KSTK_EIP(tsk) (((unsigned long *)(4096+(unsigned long)(tsk)->thread_info))[1019])
--- linux-2.5.4/arch/i386/kernel/process.c Sun Feb 10 22:00:28 2002
+++ 25/arch/i386/kernel/process.c Sun Feb 10 22:26:35 2002
@@ -55,6 +55,14 @@ asmlinkage void ret_from_fork(void) __as
int hlt_counter;

/*
+ * Return saved PC of a blocked thread.
+ */
+unsigned long thread_saved_pc(struct task_struct *tsk)
+{
+ return ((unsigned long *)tsk->thread.esp)[3];
+}
+
+/*
* Powermanagement idle function, if any..
*/
void (*pm_idle)(void);


-

2002-02-11 06:38:45

by John Weber

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

Robert Love wrote:
> On Mon, 2002-02-11 at 01:02, John Weber wrote:
>
>
>>The function thread_saved_pc() is a mystery to me. It is declared with
>>a return type of unsigned long, and yet return this:
>>
>>((unsigned long *)tsk->thread->esp)[3]
>>
>>This is confusing to me in many ways:
>>- the "thread" member of task struct is not a pointer
>>- esp is of type unsigned long, so I don't understand the cast, and
>>I certainly don't understand the [3] here.
>>
>>Can anyone explain this code to me?
>>
>
> The problem is an interdependency between processor.h and sched.h.
>
> The old code was the same, except it did
>
> t->esp
>
> where t was a thread_struct, instead of what we do now
>
> t->thread->esp
>
> where t is a task_struct. And thus whereby before we passed
>
> p->thread
>

I understand all this, but thread is not a pointer.
So shouldn't it be t->thread.esp ?

> as the argument, now you pass just `p'. I.e., its the same net-affect.
> The error is because the function needs access to both task_struct (in
> sched.h) and thread_struct (in processor.h) but the two are interrelated
> so we can't include them in each other.

Hmm... OK.

> The contents of esp is a memory address, so typecasting it to (unsigned
> long *) is OK.
>
> As for the [3], p[3] is the same as
> *(p+3)
> ie,
> *(p+sizeof(p))
> so that is legal.

*(p + (3*sizeof(p))) ?

I understand the syntax, but I don't understand why one would want to
return the address of something 3 longs away. What is this function
supposed to be doing?



2002-02-11 06:50:18

by Robert Love

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

On Mon, 2002-02-11 at 01:36, John Weber wrote:

> I understand all this, but thread is not a pointer.
> So shouldn't it be t->thread.esp ?

You are right, I missed this.

The fix is to change it to `((unsigned long *)tsk->thread.esp)[3]' but
we also still have to fix the dependency problem, which means moving it
out of processor.h. Andrew Morton just posted a patch to move it to
process.c, I think he has it right. Give that a try.

> *(p + (3*sizeof(p))) ?

Right ;)

Robert Love

2002-02-11 06:50:28

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

"David S. Miller" wrote:
>
> From: Andrew Morton <[email protected]>
> Date: Sun, 10 Feb 2002 22:29:58 -0800
>
> John Weber wrote:
> > I don't know what the problem is, but un-inlining this function isn't
> > correcting it.
>
> Try this:
>
> Not sufficient, you have to also add a dummy "struct task_struct;"
> declaration before the thread_saved_pc extern.

It's already there, line 425?

-

2002-02-11 06:54:38

by David Miller

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

From: Andrew Morton <[email protected]>
Date: Sun, 10 Feb 2002 22:48:57 -0800

"David S. Miller" wrote:
> Not sufficient, you have to also add a dummy "struct task_struct;"
> declaration before the thread_saved_pc extern.

It's already there, line 425?

Yep, my error.

2002-02-11 07:13:13

by David Miller

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

From: Andrew Morton <[email protected]>
Date: Sun, 10 Feb 2002 22:29:58 -0800

John Weber wrote:
> I don't know what the problem is, but un-inlining this function isn't
> correcting it.

Try this:

Not sufficient, you have to also add a dummy "struct task_struct;"
declaration before the thread_saved_pc extern.

2002-02-11 15:48:33

by Pascal Schmidt

[permalink] [raw]
Subject: Re: 2.5.4 Compile Error

On Mon, 11 Feb 2002 07:50:06 +0100, you wrote in linux.kernel:

> I understand the syntax, but I don't understand why one would want to
> return the address of something 3 longs away. What is this function
> supposed to be doing?

Well, from the name thread_saved_pc() it tries to get some value of PC
(the program counter ;) that's saved by that thread, right? My guess
would be that the desired value is stored on the stack at esp[3].

--
Ciao,
Pascal