2013-03-12 19:39:46

by Mike Travis

[permalink] [raw]
Subject: [PATCH 05/14] KDB: add more exports for supporting KDB modules

This patch adds some important KDB functions to be externally
usable by loadable KDB modules. Note that often drivers bring
in KDB modules for debugging, and in the past KDB has not been
limited to use by GPL only modules. This patch restores KDB
usefullness to non-GPL modules.

Cc: Tim Bird <[email protected]>
Cc: Anton Vorontsov <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Cong Wang <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Eric W. Biederman <[email protected]>
Cc: Serge Hallyn <[email protected]>
Reviewed-by: Dimitri Sivanich <[email protected]>
Signed-off-by: Mike Travis <[email protected]>
---
kernel/debug/kdb/kdb_io.c | 5 ++++-
kernel/debug/kdb/kdb_main.c | 21 ++++++++++++++++++---
kernel/debug/kdb/kdb_support.c | 17 +++++++++++++++++
kernel/kallsyms.c | 9 +++++----
kernel/signal.c | 5 +++--
5 files changed, 47 insertions(+), 10 deletions(-)

--- linux.orig/kernel/debug/kdb/kdb_io.c
+++ linux/kernel/debug/kdb/kdb_io.c
@@ -30,6 +30,7 @@
char kdb_prompt_str[CMD_BUFLEN];

int kdb_trap_printk;
+EXPORT_SYMBOL(kdb_trap_printk);

static int kgdb_transition_check(char *buffer)
{
@@ -447,6 +448,7 @@ char *kdb_getstr(char *buffer, size_t bu
kdb_nextline = 1; /* Prompt and input resets line number */
return kdb_read(buffer, bufsize);
}
+EXPORT_SYMBOL(kdb_getstr);

/*
* kdb_input_flush
@@ -839,6 +841,7 @@ kdb_print_out:
preempt_enable();
return retlen;
}
+EXPORT_SYMBOL(vkdb_printf);

int kdb_printf(const char *fmt, ...)
{
@@ -851,4 +854,4 @@ int kdb_printf(const char *fmt, ...)

return r;
}
-EXPORT_SYMBOL_GPL(kdb_printf);
+EXPORT_SYMBOL(kdb_printf);
--- linux.orig/kernel/debug/kdb/kdb_main.c
+++ linux/kernel/debug/kdb/kdb_main.c
@@ -53,19 +53,23 @@ int kdb_grep_trailing;
* Kernel debugger state flags
*/
int kdb_flags;
+EXPORT_SYMBOL(kdb_flags);
atomic_t kdb_event;
+EXPORT_SYMBOL(kdb_event);

/*
* kdb_lock protects updates to kdb_initial_cpu. Used to
* single thread processors through the kernel debugger.
*/
int kdb_initial_cpu = -1; /* cpu number that owns kdb */
+EXPORT_SYMBOL(kdb_initial_cpu);
int kdb_nextline = 1;
int kdb_state; /* General KDB state */

struct task_struct *kdb_current_task;
EXPORT_SYMBOL(kdb_current_task);
struct pt_regs *kdb_current_regs;
+EXPORT_SYMBOL(kdb_current_regs);

const char *kdb_diemsg;
static int kdb_go_count;
@@ -186,6 +190,7 @@ struct task_struct *kdb_curr_task(int cp
#endif
return p;
}
+EXPORT_SYMBOL(kdb_curr_task);

/*
* kdbgetenv - This function will return the character string value of
@@ -217,6 +222,7 @@ char *kdbgetenv(const char *match)
}
return NULL;
}
+EXPORT_SYMBOL(kdbgetenv);

/*
* kdballocenv - This function is used to allocate bytes for
@@ -293,6 +299,7 @@ int kdbgetintenv(const char *match, int
*value = (int) val;
return diag;
}
+EXPORT_SYMBOL(kdbgetintenv);

/*
* kdbgetularg - This function will convert a numeric string into an
@@ -325,6 +332,7 @@ int kdbgetularg(const char *arg, unsigne

return 0;
}
+EXPORT_SYMBOL(kdbgetularg);

int kdbgetu64arg(const char *arg, u64 *value)
{
@@ -344,6 +352,7 @@ int kdbgetu64arg(const char *arg, u64 *v

return 0;
}
+EXPORT_SYMBOL(kdbgetu64arg);

/*
* kdb_set - This function implements the 'set' command. Alter an
@@ -425,6 +434,7 @@ int kdb_set(int argc, const char **argv)

return KDB_ENVFULL;
}
+EXPORT_SYMBOL(kdb_set);

static int kdb_check_regs(void)
{
@@ -585,6 +595,7 @@ int kdbgetaddrarg(int argc, const char *

return 0;
}
+EXPORT_SYMBOL(kdbgetaddrarg);

static void kdb_cmderror(int diag)
{
@@ -1049,6 +1060,7 @@ int kdb_parse(const char *cmdstr)
return 0;
}
}
+EXPORT_SYMBOL(kdb_parse);


static int handle_ctrl_cmd(char *cmd)
@@ -1109,6 +1121,7 @@ void kdb_set_current_task(struct task_st
}
kdb_current_regs = NULL;
}
+EXPORT_SYMBOL(kdb_set_current_task);

/*
* kdb_local - The main code for kdb. This routine is invoked on a
@@ -2249,6 +2262,7 @@ void kdb_ps_suppressed(void)
kdb_printf(" suppressed,\nuse 'ps A' to see all.\n");
}
}
+EXPORT_SYMBOL(kdb_ps_suppressed);

/*
* kdb_ps - This function implements the 'ps' command which shows a
@@ -2281,6 +2295,7 @@ void kdb_ps1(const struct task_struct *p
}
}
}
+EXPORT_SYMBOL(kdb_ps1);

static int kdb_ps(int argc, const char **argv)
{
@@ -2697,7 +2712,7 @@ int kdb_register_repeat(char *cmd,

return 0;
}
-EXPORT_SYMBOL_GPL(kdb_register_repeat);
+EXPORT_SYMBOL(kdb_register_repeat);


/*
@@ -2721,7 +2736,7 @@ int kdb_register(char *cmd,
return kdb_register_repeat(cmd, func, usage, help, minlen,
KDB_REPEAT_NONE);
}
-EXPORT_SYMBOL_GPL(kdb_register);
+EXPORT_SYMBOL(kdb_register);

/*
* kdb_unregister - This function is used to unregister a kernel
@@ -2750,7 +2765,7 @@ int kdb_unregister(char *cmd)
/* Couldn't find it. */
return 1;
}
-EXPORT_SYMBOL_GPL(kdb_unregister);
+EXPORT_SYMBOL(kdb_unregister);

/* Initialize the kdb command table. */
static void __init kdb_inittab(void)
--- linux.orig/kernel/debug/kdb/kdb_support.c
+++ linux/kernel/debug/kdb/kdb_support.c
@@ -157,6 +157,7 @@ out:
debug_kfree(knt1);
return ret;
}
+EXPORT_SYMBOL(kdbnearsym);

void kdbnearsym_cleanup(void)
{
@@ -168,6 +169,7 @@ void kdbnearsym_cleanup(void)
}
}
}
+EXPORT_SYMBOL(kdbnearsym_cleanup);

static char ks_namebuf[KSYM_NAME_LEN+1], ks_namebuf_prev[KSYM_NAME_LEN+1];

@@ -214,6 +216,7 @@ int kallsyms_symbol_complete(char *prefi
memcpy(prefix_name, ks_namebuf_prev, prev_len+1);
return number;
}
+EXPORT_SYMBOL(kallsyms_symbol_complete);

/*
* kallsyms_symbol_next
@@ -242,6 +245,7 @@ int kallsyms_symbol_next(char *prefix_na
}
return 0;
}
+EXPORT_SYMBOL(kallsyms_symbol_next);

/*
* kdb_symbol_print - Standard method for printing a symbol name and offset.
@@ -292,6 +296,7 @@ void kdb_symbol_print(unsigned long addr
if (punc & KDB_SP_NEWLINE)
kdb_printf("\n");
}
+EXPORT_SYMBOL(kdb_symbol_print);

/*
* kdb_strdup - kdb equivalent of strdup, for disasm code.
@@ -312,6 +317,7 @@ char *kdb_strdup(const char *str, gfp_t
return NULL;
return strcpy(s, str);
}
+EXPORT_SYMBOL(kdb_strdup);

/*
* kdb_getarea_size - Read an area of data. The kdb equivalent of
@@ -337,6 +343,7 @@ int kdb_getarea_size(void *res, unsigned
}
return ret;
}
+EXPORT_SYMBOL(kdb_getarea_size);

/*
* kdb_putarea_size - Write an area of data. The kdb equivalent of
@@ -362,6 +369,7 @@ int kdb_putarea_size(unsigned long addr,
}
return ret;
}
+EXPORT_SYMBOL(kdb_putarea_size);

/*
* kdb_getphys - Read data from a physical address. Validate the
@@ -439,6 +447,7 @@ int kdb_getphysword(unsigned long *word,
}
return diag;
}
+EXPORT_SYMBOL(kdb_getphysword);

/*
* kdb_getword - Read a binary value. Unlike kdb_getarea, this treats
@@ -488,6 +497,7 @@ int kdb_getword(unsigned long *word, uns
}
return diag;
}
+EXPORT_SYMBOL(kdb_getword);

/*
* kdb_putword - Write a binary value. Unlike kdb_putarea, this
@@ -532,6 +542,7 @@ int kdb_putword(unsigned long addr, unsi
}
return diag;
}
+EXPORT_SYMBOL(kdb_putword);

/*
* kdb_task_state_string - Convert a string containing any of the
@@ -681,6 +692,7 @@ void kdb_print_nameval(const char *name,
else
kdb_printf("0x%lx\n", val);
}
+EXPORT_SYMBOL(kdb_print_nameval);

/* Last ditch allocator for debugging, so we can still debug even when
* the GFP_ATOMIC pool has been exhausted. The algorithms are tuned
@@ -799,6 +811,7 @@ out:
spin_unlock(&dap_lock);
return p;
}
+EXPORT_SYMBOL(debug_kmalloc);

void debug_kfree(void *p)
{
@@ -858,6 +871,7 @@ void debug_kfree(void *p)
}
spin_unlock(&dap_lock);
}
+EXPORT_SYMBOL(debug_kfree);

void debug_kusage(void)
{
@@ -907,6 +921,7 @@ void debug_kusage(void)
out:
spin_unlock(&dap_lock);
}
+EXPORT_SYMBOL(debug_kusage);

/* Maintain a small stack of kdb_flags to allow recursion without disturbing
* the global kdb state.
@@ -919,9 +934,11 @@ void kdb_save_flags(void)
BUG_ON(kdb_flags_index >= ARRAY_SIZE(kdb_flags_stack));
kdb_flags_stack[kdb_flags_index++] = kdb_flags;
}
+EXPORT_SYMBOL(kdb_save_flags);

void kdb_restore_flags(void)
{
BUG_ON(kdb_flags_index <= 0);
kdb_flags = kdb_flags_stack[--kdb_flags_index];
}
+EXPORT_SYMBOL(kdb_restore_flags);
--- linux.orig/kernel/kallsyms.c
+++ linux/kernel/kallsyms.c
@@ -183,7 +183,7 @@ unsigned long kallsyms_lookup_name(const
}
return module_kallsyms_lookup_name(name);
}
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
+EXPORT_SYMBOL(kallsyms_lookup_name);

int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
@@ -202,7 +202,7 @@ int kallsyms_on_each_symbol(int (*fn)(vo
}
return module_kallsyms_on_each_symbol(fn, data);
}
-EXPORT_SYMBOL_GPL(kallsyms_on_each_symbol);
+EXPORT_SYMBOL(kallsyms_on_each_symbol);

static unsigned long get_symbol_pos(unsigned long addr,
unsigned long *symbolsize,
@@ -384,7 +384,7 @@ int sprint_symbol(char *buffer, unsigned
{
return __sprint_symbol(buffer, address, 0, 1);
}
-EXPORT_SYMBOL_GPL(sprint_symbol);
+EXPORT_SYMBOL(sprint_symbol);

/**
* sprint_symbol_no_offset - Look up a kernel symbol and return it in a text buffer
@@ -401,7 +401,7 @@ int sprint_symbol_no_offset(char *buffer
{
return __sprint_symbol(buffer, address, 0, 0);
}
-EXPORT_SYMBOL_GPL(sprint_symbol_no_offset);
+EXPORT_SYMBOL(sprint_symbol_no_offset);

/**
* sprint_backtrace - Look up a backtrace symbol and return it in a text buffer
@@ -587,6 +587,7 @@ const char *kdb_walk_kallsyms(loff_t *po
return kdb_walk_kallsyms_iter.name;
}
}
+EXPORT_SYMBOL(kdb_walk_kallsyms);
#endif /* CONFIG_KGDB_KDB */

static const struct file_operations kallsyms_operations = {
--- linux.orig/kernel/signal.c
+++ linux/kernel/signal.c
@@ -1419,7 +1419,7 @@ out_unlock:
rcu_read_unlock();
return ret;
}
-EXPORT_SYMBOL_GPL(kill_pid_info_as_cred);
+EXPORT_SYMBOL(kill_pid_info_as_cred);

/*
* kill_something_info() interprets pid in interesting ways just like kill(2).
@@ -2491,7 +2491,7 @@ out:
}

EXPORT_SYMBOL(recalc_sigpending);
-EXPORT_SYMBOL_GPL(dequeue_signal);
+EXPORT_SYMBOL(dequeue_signal);
EXPORT_SYMBOL(flush_signals);
EXPORT_SYMBOL(force_sig);
EXPORT_SYMBOL(send_sig);
@@ -3661,4 +3661,5 @@ kdb_send_sig_info(struct task_struct *t,
else
kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
}
+EXPORT_SYMBOL(kdb_send_sig_info);
#endif /* CONFIG_KGDB_KDB */

--


2013-03-12 20:09:57

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

Mike Travis <[email protected]> writes:

> This patch adds some important KDB functions to be externally
> usable by loadable KDB modules. Note that often drivers bring
> in KDB modules for debugging, and in the past KDB has not been
> limited to use by GPL only modules. This patch restores KDB
> usefullness to non-GPL modules.

It is not ok to change EXPORT_SYMBOL_GPL to EXPORT_SYMBOL.

The symbols you are changing to EXPORT_SYMBOL from EXPORT_SYMBOL_GPL you
should not even be messing with if your source code is not in the main
kernel tree.

This patch is totally not ok.

I don't know what past you are referring to but you are changing symbols
that have never been exported as anything other than EXPORT_SYMBOL_GPL
to EXPORT_SYMBOL. The past I remember is the past where kdb was not in
the kernel tree at all.

Please go back to the drawing board and come back with a solution where
you are working with the community instead of trying asking the rest of
us to support something you won't share.

Nacked-by: "Eric W. Biederman" <[email protected]>

> --- linux.orig/kernel/signal.c
> +++ linux/kernel/signal.c
> @@ -1419,7 +1419,7 @@ out_unlock:
> rcu_read_unlock();
> return ret;
> }
> -EXPORT_SYMBOL_GPL(kill_pid_info_as_cred);
> +EXPORT_SYMBOL(kill_pid_info_as_cred);
>
> /*
> * kill_something_info() interprets pid in interesting ways just like kill(2).
> @@ -2491,7 +2491,7 @@ out:
> }
>
> EXPORT_SYMBOL(recalc_sigpending);
> -EXPORT_SYMBOL_GPL(dequeue_signal);
> +EXPORT_SYMBOL(dequeue_signal);
> EXPORT_SYMBOL(flush_signals);
> EXPORT_SYMBOL(force_sig);
> EXPORT_SYMBOL(send_sig);
> @@ -3661,4 +3661,5 @@ kdb_send_sig_info(struct task_struct *t,
> else
> kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
> }
> +EXPORT_SYMBOL(kdb_send_sig_info);
> #endif /* CONFIG_KGDB_KDB */

2013-03-12 20:23:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

On Tue, Mar 12, 2013 at 02:38:28PM -0500, Mike Travis wrote:
> This patch adds some important KDB functions to be externally
> usable by loadable KDB modules.

What modules would that be? Are they in this patch series?

> Note that often drivers bring in KDB modules for debugging, and in the
> past KDB has not been limited to use by GPL only modules.

It always has been, when was the in-kernel code any other way?

> This patch restores KDB usefullness to non-GPL modules.

As Eric says, you can't just do this, sorry.

greg k-h

2013-03-12 22:02:24

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

On Tue, 12 Mar 2013, Mike Travis wrote:
> This patch adds some important KDB functions to be externally
> usable by loadable KDB modules. Note that often drivers bring
> in KDB modules for debugging, and in the past KDB has not been
> limited to use by GPL only modules. This patch restores KDB
> usefullness to non-GPL modules.

Which past? We only care about Linus git tree as THE past.

> -EXPORT_SYMBOL_GPL(kdb_register);
> +EXPORT_SYMBOL(kdb_register);

AFAICT that function has never been an non GPL symbol in Linus
tree. Whatever the original out of tree kdb stuff used is totally
irrelevant.

Stop trying to resolve your companys or your companys customers legal
issues by false claims.

The GPL is there for a reason.

Aside of that the whole attempt to export stuff which has been not
exported before without the _GPL extension is also:

Nacked-by: Thomas Gleixner <[email protected]>

Thanks,

tglx

2013-03-12 22:03:23

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

Let me see if I can understand the concept better. By denying
an external hardware vendor the use of KDB to support a significant
piece of proprietary hardware on Linux, I furthering the interests
of Linux and the community how?

Looking back at the KDB sources originally posted on oss.sgi.com I
did not see any restrictions on the use of KDB. How/why was that
restriction granted and by whom? Was SGI, the original copyright
owner of KDB, asked or even informed of that decision? I'm not
trying to be a lawyer here, but someone decided (perhaps wrongly)
that KDB should only be used by GPL modules.

I'm not married to this matter by any means and I will change them all
if that's what's needed for acceptance. But I do think that placing
unnecessary roadblocks in the path of developing more capabilities
for the Linux system, is causing a disservice to the the users of
Linux and the overall Linux community.

Thanks,
Mike

On 3/12/2013 1:09 PM, Eric W. Biederman wrote:
> Mike Travis <[email protected]> writes:
>
>> This patch adds some important KDB functions to be externally
>> usable by loadable KDB modules. Note that often drivers bring
>> in KDB modules for debugging, and in the past KDB has not been
>> limited to use by GPL only modules. This patch restores KDB
>> usefullness to non-GPL modules.
>
> It is not ok to change EXPORT_SYMBOL_GPL to EXPORT_SYMBOL.
>
> The symbols you are changing to EXPORT_SYMBOL from EXPORT_SYMBOL_GPL you
> should not even be messing with if your source code is not in the main
> kernel tree.
>
> This patch is totally not ok.
>
> I don't know what past you are referring to but you are changing symbols
> that have never been exported as anything other than EXPORT_SYMBOL_GPL
> to EXPORT_SYMBOL. The past I remember is the past where kdb was not in
> the kernel tree at all.
>
> Please go back to the drawing board and come back with a solution where
> you are working with the community instead of trying asking the rest of
> us to support something you won't share.
>
> Nacked-by: "Eric W. Biederman" <[email protected]>
>
>> --- linux.orig/kernel/signal.c
>> +++ linux/kernel/signal.c
>> @@ -1419,7 +1419,7 @@ out_unlock:
>> rcu_read_unlock();
>> return ret;
>> }
>> -EXPORT_SYMBOL_GPL(kill_pid_info_as_cred);
>> +EXPORT_SYMBOL(kill_pid_info_as_cred);
>>
>> /*
>> * kill_something_info() interprets pid in interesting ways just like kill(2).
>> @@ -2491,7 +2491,7 @@ out:
>> }
>>
>> EXPORT_SYMBOL(recalc_sigpending);
>> -EXPORT_SYMBOL_GPL(dequeue_signal);
>> +EXPORT_SYMBOL(dequeue_signal);
>> EXPORT_SYMBOL(flush_signals);
>> EXPORT_SYMBOL(force_sig);
>> EXPORT_SYMBOL(send_sig);
>> @@ -3661,4 +3661,5 @@ kdb_send_sig_info(struct task_struct *t,
>> else
>> kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
>> }
>> +EXPORT_SYMBOL(kdb_send_sig_info);
>> #endif /* CONFIG_KGDB_KDB */

2013-03-12 22:08:56

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules



On 3/12/2013 3:01 PM, Thomas Gleixner wrote:
> On Tue, 12 Mar 2013, Mike Travis wrote:
>> This patch adds some important KDB functions to be externally
>> usable by loadable KDB modules. Note that often drivers bring
>> in KDB modules for debugging, and in the past KDB has not been
>> limited to use by GPL only modules. This patch restores KDB
>> usefullness to non-GPL modules.
>
> Which past? We only care about Linus git tree as THE past.
>
>> -EXPORT_SYMBOL_GPL(kdb_register);
>> +EXPORT_SYMBOL(kdb_register);
>
> AFAICT that function has never been an non GPL symbol in Linus
> tree. Whatever the original out of tree kdb stuff used is totally
> irrelevant.
>
> Stop trying to resolve your companys or your companys customers legal
> issues by false claims.
>
> The GPL is there for a reason.
>
> Aside of that the whole attempt to export stuff which has been not
> exported before without the _GPL extension is also:
>
> Nacked-by: Thomas Gleixner <[email protected]>
>
> Thanks,
>
> tglx
>

No problem, as I mentioned I don't care nor am I trying to resolve
anything except the problems of running Linux on the UV system.
There is no hidden agenda.

I will wait though for more feedback on the other patches before
submitting a 'v2' version.

Thanks,
Mike

2013-03-12 22:13:43

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

On Tue, Mar 12, 2013 at 03:03:17PM -0700, Mike Travis wrote:
> Let me see if I can understand the concept better. By denying
> an external hardware vendor the use of KDB to support a significant
> piece of proprietary hardware on Linux, I furthering the interests
> of Linux and the community how?

Did SGI lawyers really agree to this patch? I consider you running this
by them if you have any questions as to why we are objecting to this.
If, after discussing it with them, they still are asking for this
change, please resend it, with their signed-off-by: on it showing that
they really want this change.

greg k-h

2013-03-12 22:27:01

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules



On 3/12/2013 3:13 PM, Greg Kroah-Hartman wrote:
> On Tue, Mar 12, 2013 at 03:03:17PM -0700, Mike Travis wrote:
>> Let me see if I can understand the concept better. By denying
>> an external hardware vendor the use of KDB to support a significant
>> piece of proprietary hardware on Linux, I furthering the interests
>> of Linux and the community how?
>
> Did SGI lawyers really agree to this patch? I consider you running this
> by them if you have any questions as to why we are objecting to this.
> If, after discussing it with them, they still are asking for this
> change, please resend it, with their signed-off-by: on it showing that
> they really want this change.
>
> greg k-h
>

There is nobody else involved believe me. I am just trying to do
the right thing. This is not that big an issue as it has absolutely
no relevance to anything within that patch set. I'm trying to
improve the overall experience of using KDB, which I've found most
helpful in the past to get around some very thorny issues, particularly
in regards to bringing up new hardware. If blocking that usage by
non-GPL modules is what's required, then by all means I'm for it.

But understanding more of why the restriction is in place, would be
very helpful the next time I encounter it.

Thanks,
Mike

2013-03-12 22:39:47

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

Mike Travis <[email protected]> writes:

> Let me see if I can understand the concept better. By denying
> an external hardware vendor the use of KDB to support a significant
> piece of proprietary hardware on Linux, I furthering the interests
> of Linux and the community how?

By ignoring interests of someone who does not coopearte with the
community we encourage people to cooperate with the community.

> Looking back at the KDB sources originally posted on oss.sgi.com I
> did not see any restrictions on the use of KDB. How/why was that
> restriction granted and by whom? Was SGI, the original copyright
> owner of KDB, asked or even informed of that decision? I'm not
> trying to be a lawyer here, but someone decided (perhaps wrongly)
> that KDB should only be used by GPL modules.

The symbols quoted below are have absolutely nothing to do with KDB
ever. They are pieces of code that you should only use in very
exceptional circumpstances, or you risk breaking the kernel in strange
and mysterious ways.

Beyond that there are modules with GPL compatible licenses. That is the
only kind of module that the kernel license allows.

> I'm not married to this matter by any means and I will change them all
> if that's what's needed for acceptance. But I do think that placing
> unnecessary roadblocks in the path of developing more capabilities
> for the Linux system, is causing a disservice to the the users of
> Linux and the overall Linux community.

A capability that no one else can use, and that generates support
requests that can not be supported is not developing more capabilities
for the Linux system. It is denying those of us who ask for repayment
in code, our compensation. It is theft.

Eric

>>> --- linux.orig/kernel/signal.c
>>> +++ linux/kernel/signal.c
>>> @@ -1419,7 +1419,7 @@ out_unlock:
>>> rcu_read_unlock();
>>> return ret;
>>> }
>>> -EXPORT_SYMBOL_GPL(kill_pid_info_as_cred);
>>> +EXPORT_SYMBOL(kill_pid_info_as_cred);
>>>
>>> /*
>>> * kill_something_info() interprets pid in interesting ways just like kill(2).
>>> @@ -2491,7 +2491,7 @@ out:
>>> }
>>>
>>> EXPORT_SYMBOL(recalc_sigpending);
>>> -EXPORT_SYMBOL_GPL(dequeue_signal);
>>> +EXPORT_SYMBOL(dequeue_signal);
>>> EXPORT_SYMBOL(flush_signals);
>>> EXPORT_SYMBOL(force_sig);
>>> EXPORT_SYMBOL(send_sig);
>>> @@ -3661,4 +3661,5 @@ kdb_send_sig_info(struct task_struct *t,
>>> else
>>> kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
>>> }
>>> +EXPORT_SYMBOL(kdb_send_sig_info);
>>> #endif /* CONFIG_KGDB_KDB */

2013-03-12 23:03:28

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules



On 3/12/2013 3:39 PM, Eric W. Biederman wrote:
> Mike Travis <[email protected]> writes:
>
>> Let me see if I can understand the concept better. By denying
>> an external hardware vendor the use of KDB to support a significant
>> piece of proprietary hardware on Linux, I furthering the interests
>> of Linux and the community how?
>
> By ignoring interests of someone who does not cooperate with the
> community we encourage people to cooperate with the community.

I can see this point.
>
>> Looking back at the KDB sources originally posted on oss.sgi.com I
>> did not see any restrictions on the use of KDB. How/why was that
>> restriction granted and by whom? Was SGI, the original copyright
>> owner of KDB, asked or even informed of that decision? I'm not
>> trying to be a lawyer here, but someone decided (perhaps wrongly)
>> that KDB should only be used by GPL modules.
>
> The symbols quoted below are have absolutely nothing to do with KDB
> ever. They are pieces of code that you should only use in very
> exceptional circumpstances, or you risk breaking the kernel in strange
> and mysterious ways.

Yes, those below were indeed a mistake on my part. Thanks for catching that.
>
> Beyond that there are modules with GPL compatible licenses. That is the
> only kind of module that the kernel license allows.

Okay.
>
>> I'm not married to this matter by any means and I will change them all
>> if that's what's needed for acceptance. But I do think that placing
>> unnecessary roadblocks in the path of developing more capabilities
>> for the Linux system, is causing a disservice to the the users of
>> Linux and the overall Linux community.
>
> A capability that no one else can use, and that generates support
> requests that can not be supported is not developing more capabilities
> for the Linux system. It is denying those of us who ask for repayment
> in code, our compensation. It is theft.

Not sure I've ever looked at it this way, but again I can see your point.
>
> Eric

Thanks for the meaningful feedback Eric.

Mike

>
>>>> --- linux.orig/kernel/signal.c
>>>> +++ linux/kernel/signal.c
>>>> @@ -1419,7 +1419,7 @@ out_unlock:
>>>> rcu_read_unlock();
>>>> return ret;
>>>> }
>>>> -EXPORT_SYMBOL_GPL(kill_pid_info_as_cred);
>>>> +EXPORT_SYMBOL(kill_pid_info_as_cred);
>>>>
>>>> /*
>>>> * kill_something_info() interprets pid in interesting ways just like kill(2).
>>>> @@ -2491,7 +2491,7 @@ out:
>>>> }
>>>>
>>>> EXPORT_SYMBOL(recalc_sigpending);
>>>> -EXPORT_SYMBOL_GPL(dequeue_signal);
>>>> +EXPORT_SYMBOL(dequeue_signal);
>>>> EXPORT_SYMBOL(flush_signals);
>>>> EXPORT_SYMBOL(force_sig);
>>>> EXPORT_SYMBOL(send_sig);
>>>> @@ -3661,4 +3661,5 @@ kdb_send_sig_info(struct task_struct *t,
>>>> else
>>>> kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
>>>> }
>>>> +EXPORT_SYMBOL(kdb_send_sig_info);
>>>> #endif /* CONFIG_KGDB_KDB */

2013-03-13 15:24:28

by Scott Lurndal

[permalink] [raw]
Subject: Re: [PATCH 05/14] KDB: add more exports for supporting KDB modules

> Looking back at the KDB sources originally posted on oss.sgi.com I
> did not see any restrictions on the use of KDB. How/why was that
> restriction granted and by whom? Was SGI, the original copyright
> owner of KDB, asked or even informed of that decision? I'm not
> trying to be a lawyer here, but someone decided (perhaps wrongly)
> that KDB should only be used by GPL modules.

At the time that I wrote that original KDB code at SGI, there was no
intent to make it only usable by GPL modules. If anything, it was intended
to be more in the BSD-style (anyone can use it any way they like with
attribution) than the GPLv3-style (use it our way, or else).

But then, EXPORT_SYMBOL_GPL didn't exist then. And I think its existence
is silliness on the part of several hard-core developers.

scott lurndal