2011-06-07 22:27:44

by Andi Kleen

[permalink] [raw]
Subject: [PATCH 1/5] SH: drop const from io port variiable

From: Andi Kleen <[email protected]>

The variable is modified elsewhere, so it should not be const.
If it was really const it would need to be __read_mostly_const.

Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
arch/sh/kernel/ioport.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/sh/kernel/ioport.c b/arch/sh/kernel/ioport.c
index e3ad610..cca14ba 100644
--- a/arch/sh/kernel/ioport.c
+++ b/arch/sh/kernel/ioport.c
@@ -11,7 +11,7 @@
#include <linux/module.h>
#include <linux/io.h>

-const unsigned long sh_io_port_base __read_mostly = -1;
+unsigned long sh_io_port_base __read_mostly = -1;
EXPORT_SYMBOL(sh_io_port_base);

void __iomem *__ioport_map(unsigned long addr, unsigned int size)
--
1.7.4.4


2011-06-07 22:27:49

by Andi Kleen

[permalink] [raw]
Subject: [PATCH 2/5] Fix mismatched variable in rcutree_trace.c

From: Andi Kleen <[email protected]>

rcutree.c defines rcu_cpu_kthread_cpu as int, not unsigned int,
so the extern has to follow that.

Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
kernel/rcutree_trace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
index 9678cc3..91c56e5 100644
--- a/kernel/rcutree_trace.c
+++ b/kernel/rcutree_trace.c
@@ -47,7 +47,7 @@
#include "rcutree.h"

DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_status);
-DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_cpu);
+DECLARE_PER_CPU(int, rcu_cpu_kthread_cpu);
DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_loops);
DECLARE_PER_CPU(char, rcu_cpu_has_work);

--
1.7.4.4

2011-06-07 22:27:48

by Andi Kleen

[permalink] [raw]
Subject: [PATCH 3/5] IRDA: Fix global type conflicts in net/irda/irsysctl.c

From: Andi Kleen <[email protected]>

The externs here didn't agree with the declarations in qos.c.

Better would be probably to move this into a header, but since it's
common practice to have naked externs with sysctls I left it for now.

Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
net/irda/irsysctl.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/irda/irsysctl.c b/net/irda/irsysctl.c
index d0b70da..be22ad2 100644
--- a/net/irda/irsysctl.c
+++ b/net/irda/irsysctl.c
@@ -40,9 +40,9 @@ extern int sysctl_slot_timeout;
extern int sysctl_fast_poll_increase;
extern char sysctl_devname[];
extern int sysctl_max_baud_rate;
-extern int sysctl_min_tx_turn_time;
-extern int sysctl_max_tx_data_size;
-extern int sysctl_max_tx_window;
+extern unsigned sysctl_min_tx_turn_time;
+extern unsigned sysctl_max_tx_data_size;
+extern unsigned sysctl_max_tx_window;
extern int sysctl_max_noreply_time;
extern int sysctl_warn_noreply_time;
extern int sysctl_lap_keepalive_time;
--
1.7.4.4

2011-06-07 22:27:42

by Andi Kleen

[permalink] [raw]
Subject: [PATCH 4/5] ixgbe: Fix incorrect declaration of ixgbevf_mbx_ops

From: Andi Kleen <[email protected]>

The header extern type of ixgbevf_mbx_ops disagreed with
the actual declaration. Fix this here.

This is rather scary. I haven't tested it. Did this
ever work?

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
drivers/net/ixgbevf/ixgbevf.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbevf/ixgbevf.h b/drivers/net/ixgbevf/ixgbevf.h
index b703f60..a2bbbb3 100644
--- a/drivers/net/ixgbevf/ixgbevf.h
+++ b/drivers/net/ixgbevf/ixgbevf.h
@@ -279,7 +279,7 @@ enum ixgbevf_boards {

extern struct ixgbevf_info ixgbevf_82599_vf_info;
extern struct ixgbevf_info ixgbevf_X540_vf_info;
-extern struct ixgbe_mac_operations ixgbevf_mbx_ops;
+extern struct ixgbe_mbx_operations ixgbevf_mbx_ops;

/* needed by ethtool.c */
extern char ixgbevf_driver_name[];
--
1.7.4.4

2011-06-07 22:27:43

by Andi Kleen

[permalink] [raw]
Subject: [PATCH 5/5] Avoid section type conflict in dma/ioat/dma_v3.c

From: Andi Kleen <[email protected]>

const __read_mostly is not legal and causes section type conflicts.
That's because the read.mostly section is not read only.
Simply drop the const for the read mostly fields.

Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
drivers/dma/ioat/dma_v3.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
index d845dc4..e1e052a 100644
--- a/drivers/dma/ioat/dma_v3.c
+++ b/drivers/dma/ioat/dma_v3.c
@@ -73,10 +73,10 @@
/* provide a lookup table for setting the source address in the base or
* extended descriptor of an xor or pq descriptor
*/
-static const u8 xor_idx_to_desc __read_mostly = 0xd0;
-static const u8 xor_idx_to_field[] __read_mostly = { 1, 4, 5, 6, 7, 0, 1, 2 };
-static const u8 pq_idx_to_desc __read_mostly = 0xf8;
-static const u8 pq_idx_to_field[] __read_mostly = { 1, 4, 5, 0, 1, 2, 4, 5 };
+static u8 xor_idx_to_desc __read_mostly = 0xd0;
+static u8 xor_idx_to_field[] __read_mostly = { 1, 4, 5, 6, 7, 0, 1, 2 };
+static u8 pq_idx_to_desc __read_mostly = 0xf8;
+static u8 pq_idx_to_field[] __read_mostly = { 1, 4, 5, 0, 1, 2, 4, 5 };

static dma_addr_t xor_get_src(struct ioat_raw_descriptor *descs[2], int idx)
{
--
1.7.4.4

2011-06-07 22:42:22

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 2/5] Fix mismatched variable in rcutree_trace.c



On Tue, 7 Jun 2011, Andi Kleen wrote:

> From: Andi Kleen <[email protected]>
>
> rcutree.c defines rcu_cpu_kthread_cpu as int, not unsigned int,
> so the extern has to follow that.
>
> Cc: [email protected]
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> kernel/rcutree_trace.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
> index 9678cc3..91c56e5 100644
> --- a/kernel/rcutree_trace.c
> +++ b/kernel/rcutree_trace.c
> @@ -47,7 +47,7 @@
> #include "rcutree.h"
>
> DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_status);
> -DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_cpu);
> +DECLARE_PER_CPU(int, rcu_cpu_kthread_cpu);

Can we rather fix the DEFINE to use unsigned int. Signed cpu numbers
are pointless in that context, right ?

> DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_loops);
> DECLARE_PER_CPU(char, rcu_cpu_has_work);
>
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2011-06-07 22:56:38

by Rose, Gregory V

[permalink] [raw]
Subject: RE: [PATCH 4/5] ixgbe: Fix incorrect declaration of ixgbevf_mbx_ops

> -----Original Message-----
> From: Andi Kleen [mailto:[email protected]]
> Sent: Tuesday, June 07, 2011 3:27 PM
> To: [email protected]
> Cc: Andi Kleen; Kirsher, Jeffrey T; Brandeburg, Jesse; Rose, Gregory V
> Subject: [PATCH 4/5] ixgbe: Fix incorrect declaration of ixgbevf_mbx_ops
>
> From: Andi Kleen <[email protected]>
>
> The header extern type of ixgbevf_mbx_ops disagreed with
> the actual declaration. Fix this here.
>
> This is rather scary. I haven't tested it. Did this
> ever work?
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> drivers/net/ixgbevf/ixgbevf.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ixgbevf/ixgbevf.h b/drivers/net/ixgbevf/ixgbevf.h
> index b703f60..a2bbbb3 100644
> --- a/drivers/net/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ixgbevf/ixgbevf.h
> @@ -279,7 +279,7 @@ enum ixgbevf_boards {
>
> extern struct ixgbevf_info ixgbevf_82599_vf_info;
> extern struct ixgbevf_info ixgbevf_X540_vf_info;
> -extern struct ixgbe_mac_operations ixgbevf_mbx_ops;
> +extern struct ixgbe_mbx_operations ixgbevf_mbx_ops;
>
> /* needed by ethtool.c */
> extern char ixgbevf_driver_name[];
> --
> 1.7.4.4

[Greg Rose]
Huh.

Well, mbx ops is a table of 8 pointers and mac ops is a table of 11 pointers, so the copy would have over written 3 * pointer-size words of some other memory. But the 8 ops copied would have been correct so to the extent that we called them they worked.

Now that I look at it you missed a another spot where the copy is done that uses the ixgbe_mac_operations instead of the ixgbe_mbx_operations.

I'll fix that up.

Nice catch!

- Greg

2011-06-07 23:07:23

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 5/5] Avoid section type conflict in dma/ioat/dma_v3.c

On 6/7/2011 3:26 PM, Andi Kleen wrote:
> From: Andi Kleen<[email protected]>
>
> const __read_mostly is not legal and causes section type conflicts.
> That's because the read.mostly section is not read only.
> Simply drop the const for the read mostly fields.
>
> Cc: [email protected]

Acked-by: Dan Williams <[email protected]>

I don't have any other ioatdma stuff pending so I'm happy for this to go
in through another tree.

--
Dan

2011-06-07 23:13:10

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 2/5] Fix mismatched variable in rcutree_trace.c

On Wed, Jun 08, 2011 at 12:42:17AM +0200, Thomas Gleixner wrote:
>
>
> On Tue, 7 Jun 2011, Andi Kleen wrote:
>
> > From: Andi Kleen <[email protected]>
> >
> > rcutree.c defines rcu_cpu_kthread_cpu as int, not unsigned int,
> > so the extern has to follow that.
> >
> > Cc: [email protected]
> > Signed-off-by: Andi Kleen <[email protected]>
> > ---
> > kernel/rcutree_trace.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
> > index 9678cc3..91c56e5 100644
> > --- a/kernel/rcutree_trace.c
> > +++ b/kernel/rcutree_trace.c
> > @@ -47,7 +47,7 @@
> > #include "rcutree.h"
> >
> > DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_status);
> > -DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_cpu);
> > +DECLARE_PER_CPU(int, rcu_cpu_kthread_cpu);
>
> Can we rather fix the DEFINE to use unsigned int. Signed cpu numbers
> are pointless in that context, right ?

Hmmm... Some arches have signed CPU numbers while others have unsigned
CPU numbers. I do use "-1" to mean "no CPU" in some cases (for example,
in rcu_node_kthread_setaffinity()), so I guess for consistency I should
be using "int" everywhere.

So, any plans for systems with 2,147,483,648 CPUs? ;-)

Thanx, Paul

> > DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_loops);
> > DECLARE_PER_CPU(char, rcu_cpu_has_work);
> >
> > --
> > 1.7.4.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >

2011-06-07 23:44:01

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 2/5] Fix mismatched variable in rcutree_trace.c

> Hmmm... Some arches have signed CPU numbers while others have unsigned
> CPU numbers. I do use "-1" to mean "no CPU" in some cases (for example,
> in rcu_node_kthread_setaffinity()), so I guess for consistency I should
> be using "int" everywhere.

Do I hear that "int" is ok for you? If yes I will not revise the patch
and please merge it. If it needs changes please let me know.

-Andi

2011-06-08 00:13:16

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 2/5] Fix mismatched variable in rcutree_trace.c

On Wed, Jun 08, 2011 at 01:43:56AM +0200, Andi Kleen wrote:
> > Hmmm... Some arches have signed CPU numbers while others have unsigned
> > CPU numbers. I do use "-1" to mean "no CPU" in some cases (for example,
> > in rcu_node_kthread_setaffinity()), so I guess for consistency I should
> > be using "int" everywhere.
>
> Do I hear that "int" is ok for you? If yes I will not revise the patch
> and please merge it. If it needs changes please let me know.

Yep, "int" is OK. I queued your patch and am testing it now.

Thank you for catching this!

Thanx, Paul

2011-06-08 19:04:59

by Jeff Kirsher

[permalink] [raw]
Subject: Re: [PATCH 4/5] ixgbe: Fix incorrect declaration of ixgbevf_mbx_ops

On Tue, Jun 7, 2011 at 15:56, Rose, Gregory V <[email protected]> wrote:
>> -----Original Message-----
>> From: Andi Kleen [mailto:[email protected]]
>> Sent: Tuesday, June 07, 2011 3:27 PM
>> To: [email protected]
>> Cc: Andi Kleen; Kirsher, Jeffrey T; Brandeburg, Jesse; Rose, Gregory V
>> Subject: [PATCH 4/5] ixgbe: Fix incorrect declaration of ixgbevf_mbx_ops
>>
>> From: Andi Kleen <[email protected]>
>>
>> The header extern type of ixgbevf_mbx_ops disagreed with
>> the actual declaration. Fix this here.
>>
>> This is rather scary. I haven't tested it. Did this
>> ever work?
>>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Andi Kleen <[email protected]>
>> ---
>>  drivers/net/ixgbevf/ixgbevf.h |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ixgbevf/ixgbevf.h b/drivers/net/ixgbevf/ixgbevf.h
>> index b703f60..a2bbbb3 100644
>> --- a/drivers/net/ixgbevf/ixgbevf.h
>> +++ b/drivers/net/ixgbevf/ixgbevf.h
>> @@ -279,7 +279,7 @@ enum ixgbevf_boards {
>>
>>  extern struct ixgbevf_info ixgbevf_82599_vf_info;
>>  extern struct ixgbevf_info ixgbevf_X540_vf_info;
>> -extern struct ixgbe_mac_operations ixgbevf_mbx_ops;
>> +extern struct ixgbe_mbx_operations ixgbevf_mbx_ops;
>>
>>  /* needed by ethtool.c */
>>  extern char ixgbevf_driver_name[];
>> --
>> 1.7.4.4
>
> [Greg Rose]
> Huh.
>
> Well, mbx ops is a table of 8 pointers and mac ops is a table of 11 pointers, so the copy would have over written 3 * pointer-size words of some other memory.  But the 8 ops copied would have been correct so to the extent that we called them they worked.
>
> Now that I look at it you missed a another spot where the copy is done that uses the ixgbe_mac_operations instead of the ixgbe_mbx_operations.
>
> I'll fix that up.
>
> Nice catch!
>
> - Greg
> --

I have an updated patch by Greg in my queue, which I will be
submitting through David Miller's networking tree. Thanks Andi and
Greg!

--
Cheers,
Jeff

2011-06-09 07:30:56

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 3/5] IRDA: Fix global type conflicts in net/irda/irsysctl.c


Networking patches need to be submitted to [email protected]
otherwise they won't be properly tracked in patchwork and applied.

Thanks.