[PATCH 0/5] uml: Some trivial cleanups

[PATCH 0/5] uml: Some trivial cleanups

[Jan Kiszka]

Some trivial build warning fixes and cleanups for UML.

Jan Kiszka (5):
  uml: Remove unused variable from line driver
  uml: Drop private round_down definition
  uml: Fix warning due to missing task_struct declaration
  uml: i386: Avoid redefinition of NR_syscalls
  uml: Clean up asm/system.h

 arch/um/drivers/line.c        |    1 -
 arch/um/include/asm/system.h  |    3 ---
 arch/um/kernel/skas/syscall.c |    4 ++--
 arch/um/sys-i386/asm/elf.h    |    2 ++
 arch/um/sys-x86_64/asm/elf.h  |    2 ++
 arch/um/sys-x86_64/signal.c   |    2 --
 6 files changed, 6 insertions(+), 8 deletions(-)

[PATCH 1/5] uml: Remove unused variable from line driver

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 arch/um/drivers/line.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index 7a656bd..7f7338c 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
 {
 	struct chan *chan = data;
 	struct line *line = chan->line;
-	struct tty_struct *tty;
 
 	if (line)
 		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
-- 
1.6.0.2

[PATCH 2/5] uml: Drop private round_down definition

[Jan Kiszka]

Already defined in kernel.h. The official version assumes that 'n' is
power of two - which it is in our case.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 arch/um/sys-x86_64/signal.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
index 1a899a7..07797d1 100644
--- a/arch/um/sys-x86_64/signal.c
+++ b/arch/um/sys-x86_64/signal.c
@@ -165,8 +165,6 @@ struct rt_sigframe
 	struct _fpstate fpstate;
 };
 
-#define round_down(m, n) (((m) / (n)) * (n))
-
 int setup_signal_stack_si(unsigned long stack_top, int sig,
 			  struct k_sigaction *ka, struct pt_regs * regs,
 			  siginfo_t *info, sigset_t *set)
-- 
1.6.0.2

[PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jan Kiszka]

We can't pull in linux/sched.h, so just declare the struct.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 arch/um/sys-i386/asm/elf.h   |    2 ++
 arch/um/sys-x86_64/asm/elf.h |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/um/sys-i386/asm/elf.h b/arch/um/sys-i386/asm/elf.h
index e64cd41..a979a22 100644
--- a/arch/um/sys-i386/asm/elf.h
+++ b/arch/um/sys-i386/asm/elf.h
@@ -75,6 +75,8 @@ typedef struct user_i387_struct elf_fpregset_t;
 	pr_reg[16] = PT_REGS_SS(regs);		\
 } while (0);
 
+struct task_struct;
+
 extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
 
 #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
diff --git a/arch/um/sys-x86_64/asm/elf.h b/arch/um/sys-x86_64/asm/elf.h
index 49655c8..d760967 100644
--- a/arch/um/sys-x86_64/asm/elf.h
+++ b/arch/um/sys-x86_64/asm/elf.h
@@ -95,6 +95,8 @@ typedef struct user_i387_struct elf_fpregset_t;
 	(pr_reg)[25] = 0;					\
 	(pr_reg)[26] = 0;
 
+struct task_struct;
+
 extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
 
 #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
-- 
1.6.0.2

[PATCH 4/5] uml: i386: Avoid redefinition of NR_syscalls

[Jan Kiszka]

The i386 subarch happens to pull in original NR_syscalls. Maybe we can
make that work for all host arch, but for now just avoid the clash by
using an all-upper-case name.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 arch/um/kernel/skas/syscall.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
index 4e3b820..f5173e1 100644
--- a/arch/um/kernel/skas/syscall.c
+++ b/arch/um/kernel/skas/syscall.c
@@ -10,7 +10,7 @@
 #include "sysdep/syscalls.h"
 
 extern int syscall_table_size;
-#define NR_syscalls (syscall_table_size / sizeof(void *))
+#define NR_SYSCALLS (syscall_table_size / sizeof(void *))
 
 void handle_syscall(struct uml_pt_regs *r)
 {
@@ -30,7 +30,7 @@ void handle_syscall(struct uml_pt_regs *r)
 	 * in case it's a compiler bug.
 	 */
 	syscall = UPT_SYSCALL_NR(r);
-	if ((syscall >= NR_syscalls) || (syscall < 0))
+	if ((syscall >= NR_SYSCALLS) || (syscall < 0))
 		result = -ENOSYS;
 	else result = EXECUTE_SYSCALL(syscall, regs);
 
-- 
1.6.0.2

[PATCH 5/5] uml: Clean up asm/system.h

[Jan Kiszka]

Remove duplicates and unused prototypes.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 arch/um/include/asm/system.h |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/um/include/asm/system.h b/arch/um/include/asm/system.h
index 753346e..93af1cf 100644
--- a/arch/um/include/asm/system.h
+++ b/arch/um/include/asm/system.h
@@ -3,11 +3,8 @@
 
 #include "sysdep/system.h"
 
-extern void *switch_to(void *prev, void *next, void *last);
-
 extern int get_signals(void);
 extern int set_signals(int enable);
-extern int get_signals(void);
 extern void block_signals(void);
 extern void unblock_signals(void);
 
-- 
1.6.0.2

Re: [PATCH 1/5] uml: Remove unused variable from line driver

[Amerigo Wang]

On Mon, Apr 19, 2010 at 11:53:04PM +0200, Jan Kiszka wrote:
>Signed-off-by: Jan Kiszka <jan.kiszka@web.de>


Acked-by: WANG Cong <xiyou.wangcong@gmail.com>


>---
> arch/um/drivers/line.c |    1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
>diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
>index 7a656bd..7f7338c 100644
>--- a/arch/um/drivers/line.c
>+++ b/arch/um/drivers/line.c
>@@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
> {
> 	struct chan *chan = data;
> 	struct line *line = chan->line;
>-	struct tty_struct *tty;
> 
> 	if (line)
> 		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
>-- 
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/5] uml: Drop private round_down definition

[Amerigo Wang]

On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
>Already defined in kernel.h. The official version assumes that 'n' is
>power of two - which it is in our case.
>
>Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>---
> arch/um/sys-x86_64/signal.c |    2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
>diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
>index 1a899a7..07797d1 100644
>--- a/arch/um/sys-x86_64/signal.c
>+++ b/arch/um/sys-x86_64/signal.c
>@@ -165,8 +165,6 @@ struct rt_sigframe
> 	struct _fpstate fpstate;
> };
> 
>-#define round_down(m, n) (((m) / (n)) * (n))
>-
> int setup_signal_stack_si(unsigned long stack_top, int sig,
> 			  struct k_sigaction *ka, struct pt_regs * regs,
> 			  siginfo_t *info, sigset_t *set)

Shouldn't this signal.c #include <linux/kernel.h>?

Thanks.

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Amerigo Wang]

On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>We can't pull in linux/sched.h, so just declare the struct.
>

Did you meet any build error? If yes, please include it.

Thanks.



>Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>---
> arch/um/sys-i386/asm/elf.h   |    2 ++
> arch/um/sys-x86_64/asm/elf.h |    2 ++
> 2 files changed, 4 insertions(+), 0 deletions(-)
>
>diff --git a/arch/um/sys-i386/asm/elf.h b/arch/um/sys-i386/asm/elf.h
>index e64cd41..a979a22 100644
>--- a/arch/um/sys-i386/asm/elf.h
>+++ b/arch/um/sys-i386/asm/elf.h
>@@ -75,6 +75,8 @@ typedef struct user_i387_struct elf_fpregset_t;
> 	pr_reg[16] = PT_REGS_SS(regs);		\
> } while (0);
> 
>+struct task_struct;
>+
> extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
> 
> #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
>diff --git a/arch/um/sys-x86_64/asm/elf.h b/arch/um/sys-x86_64/asm/elf.h
>index 49655c8..d760967 100644
>--- a/arch/um/sys-x86_64/asm/elf.h
>+++ b/arch/um/sys-x86_64/asm/elf.h
>@@ -95,6 +95,8 @@ typedef struct user_i387_struct elf_fpregset_t;
> 	(pr_reg)[25] = 0;					\
> 	(pr_reg)[26] = 0;
> 
>+struct task_struct;
>+
> extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
> 
> #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
>-- 
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/5] uml: i386: Avoid redefinition of NR_syscalls

[Amerigo Wang]

On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>make that work for all host arch, but for now just avoid the clash by
>using an all-upper-case name.
>

Where?


>Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>---
> arch/um/kernel/skas/syscall.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
>index 4e3b820..f5173e1 100644
>--- a/arch/um/kernel/skas/syscall.c
>+++ b/arch/um/kernel/skas/syscall.c
>@@ -10,7 +10,7 @@
> #include "sysdep/syscalls.h"
> 
> extern int syscall_table_size;
>-#define NR_syscalls (syscall_table_size / sizeof(void *))
>+#define NR_SYSCALLS (syscall_table_size / sizeof(void *))
> 
> void handle_syscall(struct uml_pt_regs *r)
> {
>@@ -30,7 +30,7 @@ void handle_syscall(struct uml_pt_regs *r)
> 	 * in case it's a compiler bug.
> 	 */
> 	syscall = UPT_SYSCALL_NR(r);
>-	if ((syscall >= NR_syscalls) || (syscall < 0))
>+	if ((syscall >= NR_SYSCALLS) || (syscall < 0))
> 		result = -ENOSYS;
> 	else result = EXECUTE_SYSCALL(syscall, regs);
> 
>-- 
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/5] uml: Clean up asm/system.h

[Amerigo Wang]

On Mon, Apr 19, 2010 at 11:53:08PM +0200, Jan Kiszka wrote:
>Remove duplicates and unused prototypes.
>
>Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

Acked-by: WANG Cong <xiyou.wangcong@gmail.com>


>---
> arch/um/include/asm/system.h |    3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
>diff --git a/arch/um/include/asm/system.h b/arch/um/include/asm/system.h
>index 753346e..93af1cf 100644
>--- a/arch/um/include/asm/system.h
>+++ b/arch/um/include/asm/system.h
>@@ -3,11 +3,8 @@
> 
> #include "sysdep/system.h"
> 
>-extern void *switch_to(void *prev, void *next, void *last);
>-
> extern int get_signals(void);
> extern int set_signals(int enable);
>-extern int get_signals(void);
> extern void block_signals(void);
> extern void unblock_signals(void);
> 
>-- 
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/5] uml: Drop private round_down definition

[Jeff Dike]

On Tue, Apr 20, 2010 at 04:33:58PM +0800, Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
> >Already defined in kernel.h. The official version assumes that 'n' is
> >power of two - which it is in our case.
> >
> >Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> >---
> > arch/um/sys-x86_64/signal.c |    2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> >index 1a899a7..07797d1 100644
> >--- a/arch/um/sys-x86_64/signal.c
> >+++ b/arch/um/sys-x86_64/signal.c
> >@@ -165,8 +165,6 @@ struct rt_sigframe
> > 	struct _fpstate fpstate;
> > };
> > 
> >-#define round_down(m, n) (((m) / (n)) * (n))
> >-
> > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > 			  struct k_sigaction *ka, struct pt_regs * regs,
> > 			  siginfo_t *info, sigset_t *set)
> 
> Shouldn't this signal.c #include <linux/kernel.h>?

I agree - if this is going to depend on kernel.h, it should be
explicitly included.

				Jeff

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jeff Dike]

On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
> >We can't pull in linux/sched.h, so just declare the struct.
> >
> 
> Did you meet any build error? If yes, please include it.

What does this patch fix, aside from being a bit cleaner?

If it built before, without having a task_struct declaration, I think
that means that the elf_core_copy_fpregs was never used.  The
task_struct * in the declaration would become a private task_struct,
known only to the declaration.  If the implementation or callers have
the regular task_struct, it will be a different one, and the
prototypes will conflict due to the different types of the first
parameter.

			Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [PATCH 1/5] uml: Remove unused variable from line driver

[Jiri Kosina]

On Mon, 19 Apr 2010, Jan Kiszka wrote:

> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> ---
>  arch/um/drivers/line.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
> index 7a656bd..7f7338c 100644
> --- a/arch/um/drivers/line.c
> +++ b/arch/um/drivers/line.c
> @@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
>  {
>  	struct chan *chan = data;
>  	struct line *line = chan->line;
> -	struct tty_struct *tty;
>  
>  	if (line)
>  		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 5/5] uml: Clean up asm/system.h

[Jiri Kosina]

On Mon, 19 Apr 2010, Jan Kiszka wrote:

> Remove duplicates and unused prototypes.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/5] uml: Drop private round_down definition

[Jiri Kosina]

On Tue, 20 Apr 2010, Amerigo Wang wrote:

> On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
> >Already defined in kernel.h. The official version assumes that 'n' is
> >power of two - which it is in our case.
> >
> >Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> >---
> > arch/um/sys-x86_64/signal.c |    2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> >index 1a899a7..07797d1 100644
> >--- a/arch/um/sys-x86_64/signal.c
> >+++ b/arch/um/sys-x86_64/signal.c
> >@@ -165,8 +165,6 @@ struct rt_sigframe
> > 	struct _fpstate fpstate;
> > };
> > 
> >-#define round_down(m, n) (((m) / (n)) * (n))
> >-
> > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > 			  struct k_sigaction *ka, struct pt_regs * regs,
> > 			  siginfo_t *info, sigset_t *set)
> 
> Shouldn't this signal.c #include <linux/kernel.h>?

Well, it gets included implicitly through uaccess.h -> sched.h -> 
kernel.h.

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/5] uml: Drop private round_down definition

[Jiri Kosina]

On Tue, 20 Apr 2010, Jeff Dike wrote:

> > >Already defined in kernel.h. The official version assumes that 'n' is
> > >power of two - which it is in our case.
> > >
> > >Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> > >---
> > > arch/um/sys-x86_64/signal.c |    2 --
> > > 1 files changed, 0 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> > >index 1a899a7..07797d1 100644
> > >--- a/arch/um/sys-x86_64/signal.c
> > >+++ b/arch/um/sys-x86_64/signal.c
> > >@@ -165,8 +165,6 @@ struct rt_sigframe
> > > 	struct _fpstate fpstate;
> > > };
> > > 
> > >-#define round_down(m, n) (((m) / (n)) * (n))
> > >-
> > > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > > 			  struct k_sigaction *ka, struct pt_regs * regs,
> > > 			  siginfo_t *info, sigset_t *set)
> > 
> > Shouldn't this signal.c #include <linux/kernel.h>?
> 
> I agree - if this is going to depend on kernel.h, it should be
> explicitly included.

Yup, I have already added that.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [uml-devel] [PATCH 2/5] uml: Drop private round_down definition

[Jeff Dike]

On Tue, Apr 20, 2010 at 04:35:52PM +0200, Jiri Kosina wrote:
> On Tue, 20 Apr 2010, Amerigo Wang wrote:
> > Shouldn't this signal.c #include <linux/kernel.h>?
> 
> Well, it gets included implicitly through uaccess.h -> sched.h -> 
> kernel.h.

You're depending on the internal details of uaccess.h and sched.h.  If
either of them changed, then this would cause unexpected compile
failures here.

Better to explicitly include kernel.h.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Jeff Dike wrote:
> On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
>> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>>> We can't pull in linux/sched.h, so just declare the struct.
>>>
>> Did you meet any build error? If yes, please include it.
> 
> What does this patch fix, aside from being a bit cleaner?

  CC      arch/um/sys-i386/elfcore.o
In file included from /data/linux-2.6/include/linux/elf.h:8,
                 from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want

I guess not many people build against i386 hosts anymore, so this
remained widely unnoticed.

> 
> If it built before, without having a task_struct declaration, I think
> that means that the elf_core_copy_fpregs was never used.  The
> task_struct * in the declaration would become a private task_struct,
> known only to the declaration.  If the implementation or callers have
> the regular task_struct, it will be a different one, and the
> prototypes will conflict due to the different types of the first
> parameter.

This is just a forward declaration (that many arch elf header include),
so no such problem exists.

BTW, to answer the other question in this thread: We have a circular
dependency that prevents including sched.h.

I can add all these information to some v2 of this patch if it is
required to get this merged. Please let me know.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 4/5] uml: i386: Avoid redefinition of NR_syscalls

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>> The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>> make that work for all host arch, but for now just avoid the clash by
>> using an all-upper-case name.
>>
> 
> Where?

Not sure if this answers your question:

  CC      arch/um/kernel/skas/syscall.o
/data/linux-2.6/arch/um/kernel/skas/syscall.c:13:1: warning:
"NR_syscalls" redefined
In file included from /data/linux-2.6/arch/x86/include/asm/unistd.h:3,
                 from
/data/linux-2.6/arch/um/sys-i386/shared/sysdep/syscalls.h:6,
                 from /data/linux-2.6/arch/um/kernel/skas/syscall.c:10:
/data/linux-2.6/arch/x86/include/asm/unistd_32.h:349:1: warning: this is
the location of the previous definition

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [uml-devel] [PATCH 2/5] uml: Drop private round_down definition

[Jiri Kosina]

On Tue, 20 Apr 2010, Jeff Dike wrote:

> > > Shouldn't this signal.c #include <linux/kernel.h>?
> > 
> > Well, it gets included implicitly through uaccess.h -> sched.h -> 
> > kernel.h.
> 
> You're depending on the internal details of uaccess.h and sched.h.  If
> either of them changed, then this would cause unexpected compile
> failures here.
> 
> Better to explicitly include kernel.h.

Yeah, I have that already in my tree.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jiri Kosina]

On Tue, 20 Apr 2010, Jan Kiszka wrote:

> >>> We can't pull in linux/sched.h, so just declare the struct.
> >>>
> >> Did you meet any build error? If yes, please include it.
> > 
> > What does this patch fix, aside from being a bit cleaner?
> 
>   CC      arch/um/sys-i386/elfcore.o
> In file included from /data/linux-2.6/include/linux/elf.h:8,
>                  from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
> /data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
> /data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
> 
> I guess not many people build against i386 hosts anymore, so this
> remained widely unnoticed.
> 
> > 
> > If it built before, without having a task_struct declaration, I think
> > that means that the elf_core_copy_fpregs was never used.  The
> > task_struct * in the declaration would become a private task_struct,
> > known only to the declaration.  If the implementation or callers have
> > the regular task_struct, it will be a different one, and the
> > prototypes will conflict due to the different types of the first
> > parameter.
> 
> This is just a forward declaration (that many arch elf header include),
> so no such problem exists.
> 
> BTW, to answer the other question in this thread: We have a circular
> dependency that prevents including sched.h.
> 
> I can add all these information to some v2 of this patch if it is
> required to get this merged. Please let me know.

I have updated the explanation in the changelog and applied the patch. If 
anyone has any objections still, please let me know.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Amerigo Wang]

On Tue, Apr 20, 2010 at 07:09:15PM +0200, Jan Kiszka wrote:
>Jeff Dike wrote:
>> On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
>>> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>>>> We can't pull in linux/sched.h, so just declare the struct.
>>>>
>>> Did you meet any build error? If yes, please include it.
>> 
>> What does this patch fix, aside from being a bit cleaner?
>
>  CC      arch/um/sys-i386/elfcore.o
>In file included from /data/linux-2.6/include/linux/elf.h:8,
>                 from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
>/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
>/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
>
>I guess not many people build against i386 hosts anymore, so this
>remained widely unnoticed.
>
>> 
>> If it built before, without having a task_struct declaration, I think
>> that means that the elf_core_copy_fpregs was never used.  The
>> task_struct * in the declaration would become a private task_struct,
>> known only to the declaration.  If the implementation or callers have
>> the regular task_struct, it will be a different one, and the
>> prototypes will conflict due to the different types of the first
>> parameter.
>
>This is just a forward declaration (that many arch elf header include),
>so no such problem exists.
>
>BTW, to answer the other question in this thread: We have a circular
>dependency that prevents including sched.h.
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is the right reason to do this. Ok then, thanks.

But it looks like x86_64 needs this too.


BTW, I don't think compile warning fixes are trivial enough to go
to trivial@kernel.org.

Re: [PATCH 4/5] uml: i386: Avoid redefinition of NR_syscalls

[Amerigo Wang]

On Tue, Apr 20, 2010 at 07:14:43PM +0200, Jan Kiszka wrote:
>Amerigo Wang wrote:
>> On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>>> The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>>> make that work for all host arch, but for now just avoid the clash by
>>> using an all-upper-case name.
>>>
>> 
>> Where?
>
>Not sure if this answers your question:
>
>  CC      arch/um/kernel/skas/syscall.o
>/data/linux-2.6/arch/um/kernel/skas/syscall.c:13:1: warning:
>"NR_syscalls" redefined
>In file included from /data/linux-2.6/arch/x86/include/asm/unistd.h:3,
>                 from
>/data/linux-2.6/arch/um/sys-i386/shared/sysdep/syscalls.h:6,
>                 from /data/linux-2.6/arch/um/kernel/skas/syscall.c:10:
>/data/linux-2.6/arch/x86/include/asm/unistd_32.h:349:1: warning: this is
>the location of the previous definition
>

Ah, sure. I misunderstood your purpose, please do include the warning
messages you are trying to fix in your patch description.


Thanks!

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jiri Kosina]

On Wed, 21 Apr 2010, Amerigo Wang wrote:

> >  CC      arch/um/sys-i386/elfcore.o
> >In file included from /data/linux-2.6/include/linux/elf.h:8,
> >                 from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
> >/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
> >/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
> >
> >I guess not many people build against i386 hosts anymore, so this
> >remained widely unnoticed.
> >
> >> 
> >> If it built before, without having a task_struct declaration, I think
> >> that means that the elf_core_copy_fpregs was never used.  The
> >> task_struct * in the declaration would become a private task_struct,
> >> known only to the declaration.  If the implementation or callers have
> >> the regular task_struct, it will be a different one, and the
> >> prototypes will conflict due to the different types of the first
> >> parameter.
> >
> >This is just a forward declaration (that many arch elf header include),
> >so no such problem exists.
> >
> >BTW, to answer the other question in this thread: We have a circular
> >dependency that prevents including sched.h.
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This is the right reason to do this. Ok then, thanks.
> 
> But it looks like x86_64 needs this too.
> 
> 
> BTW, I don't think compile warning fixes are trivial enough to go
> to trivial@kernel.org.

Why?

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Amerigo Wang]

On Wed, Apr 21, 2010 at 11:38:53AM +0200, Jiri Kosina wrote:
>On Wed, 21 Apr 2010, Amerigo Wang wrote:
>> 
>> BTW, I don't think compile warning fixes are trivial enough to go
>> to trivial@kernel.org.
>
>Why?
>

Usually compile warnings fixes go into -mm, some coding style fixes
too.

Re: [PATCH 3/5] uml: Fix warning due to missing task_struct declaration

[Jiri Kosina]

On Wed, 21 Apr 2010, Amerigo Wang wrote:

> >> BTW, I don't think compile warning fixes are trivial enough to go
> >> to trivial@kernel.org.
> >
> >Why?
> >
> Usually compile warnings fixes go into -mm, some coding style fixes
> too.

Well, I personally don't care that much, submit your patches whereever you 
wish. 

The point of trivial tree is to take off load from other maintainers 
(including Andrew) so that focusing on The Real Things is possible.

See relevant section of Documentation/SubmittingPatches.

But, as I said, I don't care that much. If you want to avoid trivial tree 
for some reason, feel free to do so.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 4/5] uml: i386: Avoid redefinition of NR_syscalls

[Jiri Kosina]

On Wed, 21 Apr 2010, Amerigo Wang wrote:

> >> Where?
> >
> >Not sure if this answers your question:
> >
> >  CC      arch/um/kernel/skas/syscall.o
> >/data/linux-2.6/arch/um/kernel/skas/syscall.c:13:1: warning:
> >"NR_syscalls" redefined
> >In file included from /data/linux-2.6/arch/x86/include/asm/unistd.h:3,
> >                 from
> >/data/linux-2.6/arch/um/sys-i386/shared/sysdep/syscalls.h:6,
> >                 from /data/linux-2.6/arch/um/kernel/skas/syscall.c:10:
> >/data/linux-2.6/arch/x86/include/asm/unistd_32.h:349:1: warning: this is
> >the location of the previous definition
> >
> 
> Ah, sure. I misunderstood your purpose, please do include the warning
> messages you are trying to fix in your patch description.

The patch doesn't seem to be present in linux-next as of today. I have 
applied it to my queue.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.