2008-08-14 06:44:42

by Jeffrey V. Merkey

[permalink] [raw]
Subject: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

export the task_curr function to the module based kernel debugger to enable
process back tracing and state display.


Signed-off-by: Jeffrey Vernon Merkey ([email protected])

--- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
+++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
@@ -1736,6 +1736,9 @@
{
return cpu_curr(task_cpu(p)) == p;
}
+#if defined(CONFIG_MDB_MODULE)
+EXPORT_SYMBOL_GPL(task_curr);
+#endif

static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
{


2008-08-14 08:39:18

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]> wrote:
> export the task_curr function to the module based kernel debugger to enable
> process back tracing and state display.
>
> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
>
> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
> @@ -1736,6 +1736,9 @@
> {
> return cpu_curr(task_cpu(p)) == p;
> }
> +#if defined(CONFIG_MDB_MODULE)
> +EXPORT_SYMBOL_GPL(task_curr);
> +#endif

We usually don't export symbols conditionally, especially in core kernel code.

2008-08-14 14:59:28

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

> On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]> wrote:
>> export the task_curr function to the module based kernel debugger to
>> enable
>> process back tracing and state display.
>>
>> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
>>
>> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>> @@ -1736,6 +1736,9 @@
>> {
>> return cpu_curr(task_cpu(p)) == p;
>> }
>> +#if defined(CONFIG_MDB_MODULE)
>> +EXPORT_SYMBOL_GPL(task_curr);
>> +#endif
>
> We usually don't export symbols conditionally, especially in core kernel
> code.
>

Well,then please suggest how a kernel debugger can be module based and still
be able to get this information some other way that's generic and minimal
impact.

Jeff

2008-08-14 15:06:20

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

[email protected] writes:

>> On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]> wrote:
>>> export the task_curr function to the module based kernel debugger to
>>> enable
>>> process back tracing and state display.
>>>
>>> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
>>>
>>> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>>> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>>> @@ -1736,6 +1736,9 @@
>>> {
>>> return cpu_curr(task_cpu(p)) == p;
>>> }
>>> +#if defined(CONFIG_MDB_MODULE)
>>> +EXPORT_SYMBOL_GPL(task_curr);
>>> +#endif
>>
>> We usually don't export symbols conditionally, especially in core kernel
>> code.
>>
>
> Well,then please suggest how a kernel debugger can be module based and still
> be able to get this information some other way that's generic and minimal
> impact.

EXPORT_SYMBOL_GPL(task_curr);

Without the #if's. Just export it.

Hannes

2008-08-14 15:13:12

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

> [email protected] writes:
>
>>> On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]>
>>> wrote:
>>>> export the task_curr function to the module based kernel debugger to
>>>> enable
>>>> process back tracing and state display.
>>>>
>>>> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
>>>>
>>>> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>>>> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>>>> @@ -1736,6 +1736,9 @@
>>>> {
>>>> return cpu_curr(task_cpu(p)) == p;
>>>> }
>>>> +#if defined(CONFIG_MDB_MODULE)
>>>> +EXPORT_SYMBOL_GPL(task_curr);
>>>> +#endif
>>>
>>> We usually don't export symbols conditionally, especially in core
>>> kernel
>>> code.
>>>
>>
>> Well,then please suggest how a kernel debugger can be module based and
>> still
>> be able to get this information some other way that's generic and
>> minimal
>> impact.
>
> EXPORT_SYMBOL_GPL(task_curr);
>
> Without the #if's. Just export it.
>
> Hannes
>

OK. That's simple and minimal.

Jeff

2008-08-15 12:25:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr


* [email protected] <[email protected]> wrote:

> > On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]> wrote:
> >> export the task_curr function to the module based kernel debugger to
> >> enable
> >> process back tracing and state display.
> >>
> >> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
> >>
> >> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
> >> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
> >> @@ -1736,6 +1736,9 @@
> >> {
> >> return cpu_curr(task_cpu(p)) == p;
> >> }
> >> +#if defined(CONFIG_MDB_MODULE)
> >> +EXPORT_SYMBOL_GPL(task_curr);
> >> +#endif
> >
> > We usually don't export symbols conditionally, especially in core kernel
> > code.
> >
>
> Well,then please suggest how a kernel debugger can be module based and
> still be able to get this information some other way that's generic
> and minimal impact.

FYI, there's a built-in kernel debugger in the upstream kernel already:
kernel/kgdb.c - and it does not need that export.

So if you are interested in kernel debuggers i'd suggest to work with
the KGDB folks to extend it with whatever feature-set is missing.

They are friendly, very easy to work with and are open to the
thousands-of-years-old scientific method of not duplicating effort,
working together, going forward gradually, etc.

Ingo

2008-08-15 16:22:46

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

>
> * [email protected] <[email protected]> wrote:
>
>> > On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]>
>> wrote:
>> >> export the task_curr function to the module based kernel debugger to
>> >> enable
>> >> process back tracing and state display.
>> >>
>> >> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
>> >>
>> >> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>> >> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>> >> @@ -1736,6 +1736,9 @@
>> >> {
>> >> return cpu_curr(task_cpu(p)) == p;
>> >> }
>> >> +#if defined(CONFIG_MDB_MODULE)
>> >> +EXPORT_SYMBOL_GPL(task_curr);
>> >> +#endif
>> >
>> > We usually don't export symbols conditionally, especially in core
>> kernel
>> > code.
>> >
>>
>> Well,then please suggest how a kernel debugger can be module based and
>> still be able to get this information some other way that's generic
>> and minimal impact.
>
> FYI, there's a built-in kernel debugger in the upstream kernel already:
> kernel/kgdb.c - and it does not need that export.
>
> So if you are interested in kernel debuggers i'd suggest to work with
> the KGDB folks to extend it with whatever feature-set is missing.
>
> They are friendly, very easy to work with and are open to the
> thousands-of-years-old scientific method of not duplicating effort,
> working together, going forward gradually, etc.
>
> Ingo
>

Sorry, but I have my own path and direction, and its not as a tin can on a
strong tied to the tail of kdb or kgdb. Again, it matters not what's in
Linus' tree or not, but whether this is useful to others and they use it.
>From what I have seen based on the non-stop downloads from my FTP server,
its needed for certain classes of users -- me included. I will continue
to enhance it, release it, and move it forward.

kdb and kgdb can go their way. I'm certain my code is being reviewed as
we speak and if its useful to them, so much the better.

Jeff





2008-08-15 16:37:49

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

>
>> >> +EXPORT_SYMBOL_GPL(task_curr);
>> >> +#endif
>> >
>> > We usually don't export symbols conditionally, especially in core
>> kernel
>> > code.
>> >
>>
>
> FYI, there's a built-in kernel debugger in the upstream kernel already:
> kernel/kgdb.c - and it does not need that export.

I do not need this export either -- unless mdb runs as a kernel module -
which kdb and kgdb cannot do today ...

Jeff

>
> Ingo
>

2008-08-15 16:39:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr


* [email protected] <[email protected]> wrote:

> >
> > * [email protected] <[email protected]> wrote:
> >
> >> > On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]>
> >> wrote:
> >> >> export the task_curr function to the module based kernel debugger to
> >> >> enable
> >> >> process back tracing and state display.
> >> >>
> >> >> Signed-off-by: Jeffrey Vernon Merkey ([email protected])
> >> >>
> >> >> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
> >> >> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
> >> >> @@ -1736,6 +1736,9 @@
> >> >> {
> >> >> return cpu_curr(task_cpu(p)) == p;
> >> >> }
> >> >> +#if defined(CONFIG_MDB_MODULE)
> >> >> +EXPORT_SYMBOL_GPL(task_curr);
> >> >> +#endif
> >> >
> >> > We usually don't export symbols conditionally, especially in core
> >> kernel
> >> > code.
> >> >
> >>
> >> Well,then please suggest how a kernel debugger can be module based and
> >> still be able to get this information some other way that's generic
> >> and minimal impact.
> >
> > FYI, there's a built-in kernel debugger in the upstream kernel already:
> > kernel/kgdb.c - and it does not need that export.
> >
> > So if you are interested in kernel debuggers i'd suggest to work with
> > the KGDB folks to extend it with whatever feature-set is missing.
> >
> > They are friendly, very easy to work with and are open to the
> > thousands-of-years-old scientific method of not duplicating effort,
> > working together, going forward gradually, etc.
> >
> > Ingo
> >
>
> Sorry, but I have my own path and direction, [...]

doing things alone is your unalienable personal right, and since thus
you apparently didnt intend your patch to be included in the Linux
kernel that's as far as my interest in this as a maintainer goes.

Thanks,

Ingo

2008-08-15 16:49:56

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

>
> * [email protected] <[email protected]> wrote:
>
>> >
>> > * [email protected] <[email protected]> wrote:
>> >
>> >> > On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]>
>> >> wrote:
>> >> >> export the task_curr function to the module based kernel debugger
>> to
>> >> >> enable
>> >> >> process back tracing and state display.
>> >> >>
>> >> >> Signed-off-by: Jeffrey Vernon Merkey
>> ([email protected])
>> >> >>
>> >> >> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>> >> >> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>> >> >> @@ -1736,6 +1736,9 @@
>> >> >> {
>> >> >> return cpu_curr(task_cpu(p)) == p;
>> >> >> }
>> >> >> +#if defined(CONFIG_MDB_MODULE)
>> >> >> +EXPORT_SYMBOL_GPL(task_curr);
>> >> >> +#endif
>> >> >
>> >> > We usually don't export symbols conditionally, especially in core
>> >> kernel
>> >> > code.
>> >> >
>> >>
>> >> Well,then please suggest how a kernel debugger can be module based
>> and
>> >> still be able to get this information some other way that's generic
>> >> and minimal impact.
>> >
>> > FYI, there's a built-in kernel debugger in the upstream kernel
>> already:
>> > kernel/kgdb.c - and it does not need that export.
>> >
>> > So if you are interested in kernel debuggers i'd suggest to work with
>> > the KGDB folks to extend it with whatever feature-set is missing.
>> >
>> > They are friendly, very easy to work with and are open to the
>> > thousands-of-years-old scientific method of not duplicating effort,
>> > working together, going forward gradually, etc.
>> >
>> > Ingo
>> >
>>
>> Sorry, but I have my own path and direction, [...]
>
> doing things alone is your unalienable personal right, and since thus
> you apparently didnt intend your patch to be included in the Linux
> kernel that's as far as my interest in this as a maintainer goes.
>
> Thanks,
>
> Ingo
>

Ingo,

The patches are submitted so you can stop putting words in my mouth. If
it goes in, it goes in -- its submitted. Your comment about this export
and comparing it to kgdb demonstrates 1) you have never even looked at the
code of either debugger in this area 2) you were unaware I did not need
this exported except as a module 3) which kgdb and kdb do not support
either.

A maintainer would be expected to actually review the code. Its clear
from the comments above you have not, so I see little interest anyway from
you, so who cares.

Jeff


2008-08-15 17:37:57

by Jeffrey V. Merkey

[permalink] [raw]
Subject: Re: [PATCH 2.6.27-rc3 26/29] mdb: export task_curr

>>
>> * [email protected] <[email protected]> wrote:
>>
>>> >
>>> > * [email protected] <[email protected]>
>>> wrote:
>>> >
>>> >> > On Thu, Aug 14, 2008 at 9:14 AM, <[email protected]>
>>> >> wrote:
>>> >> >> export the task_curr function to the module based kernel debugger
>>> to
>>> >> >> enable
>>> >> >> process back tracing and state display.
>>> >> >>
>>> >> >> Signed-off-by: Jeffrey Vernon Merkey
>>> ([email protected])
>>> >> >>
>>> >> >> --- a/kernel/sched.c 2008-08-13 14:22:32.000000000 -0600
>>> >> >> +++ b/kernel/sched.c 2008-08-13 11:56:03.000000000 -0600
>>> >> >> @@ -1736,6 +1736,9 @@
>>> >> >> {
>>> >> >> return cpu_curr(task_cpu(p)) == p;
>>> >> >> }
>>> >> >> +#if defined(CONFIG_MDB_MODULE)
>>> >> >> +EXPORT_SYMBOL_GPL(task_curr);
>>> >> >> +#endif
>>> >> >
>>> >> > We usually don't export symbols conditionally, especially in core
>>> >> kernel
>>> >> > code.
>>> >> >
>>> >>
>>> >> Well,then please suggest how a kernel debugger can be module based
>>> and
>>> >> still be able to get this information some other way that's generic
>>> >> and minimal impact.
>>> >
>>> > FYI, there's a built-in kernel debugger in the upstream kernel
>>> already:
>>> > kernel/kgdb.c - and it does not need that export.
>>> >
>>> > So if you are interested in kernel debuggers i'd suggest to work with
>>> > the KGDB folks to extend it with whatever feature-set is missing.
>>> >
>>> > They are friendly, very easy to work with and are open to the
>>> > thousands-of-years-old scientific method of not duplicating effort,
>>> > working together, going forward gradually, etc.
>>> >
>>> > Ingo
>>> >
>>>
>>> Sorry, but I have my own path and direction, [...]
>>
>> doing things alone is your unalienable personal right, and since thus
>> you apparently didnt intend your patch to be included in the Linux
>> kernel that's as far as my interest in this as a maintainer goes.
>>
>> Thanks,
>>
>> Ingo
>>


I removed the export from this file and will resubmit the patch series.
This will remove you from the decision list for this patch series. What
other files do you claim to maintain so I can make certain remove them
from my patches?

I can get the variable some other way.

:-)

Thanks

Jeff