Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:47 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <20100126023542.GQ5087@nowhere>

On 01/26/2010 11:35 AM, Frederic Weisbecker wrote:
> No guarantee that will build. I should pull your tree and install
> sparse (yeah, shame on me, I've never installed it).

Nope, it doesn't.  Please pull from the following tree to receive the
whole thing.  The definitions in question are in
include/asm-generic/percpu.h.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/percpu.git percpu-sparse-review

After installing sparse,

 make C=2 arch/x86/kernel/cpu/common.o

should be enough.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:43 UTC (permalink / raw)
  To: Al Viro
  Cc: H. Peter Anvin, Frederic Weisbecker, linux-kernel, axboe, rusty,
	akpm, ebiederm, tytso, Trond.Myklebust, aelder, hch, davem,
	netdev, x86, mingo, dan.j.williams, borislav.petkov, ying.huang,
	lenb, neilb, cl
In-Reply-To: <20100126023238.GC19799@ZenIV.linux.org.uk>

Hello,

On 01/26/2010 11:32 AM, Al Viro wrote:
> On Tue, Jan 26, 2010 at 11:16:42AM +0900, Tejun Heo wrote:
> 
>> # define RELOC_HIDE(ptr, off)					\
>>   ({ unsigned long __ptr;					\
>>      __ptr = (unsigned long) (ptr);				\
>>     (typeof(ptr)) (__ptr + (off)); })
>>
>> #define __verify_pcpu_ptr(ptr)	do {					\
>> 	const void __percpu *__vpp_verify = (typeof(ptr))NULL;		\
>> 	(void)__vpp_verify;						\
>> } while (0)
>>
>> /* Weird cast keeps both GCC and sparse happy. */
>> #define SHIFT_PERCPU_PTR(__p, __offset)	({				\
>> 	__verify_pcpu_ptr((__p));					\
>> 	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
>> })
>>
>> #define per_cpu(var, cpu) \
>> 	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 
> Eh...  You are leaving that noderef in place in case of array.  And _that_
> is not an address space, so casts to AS 0 won't do you any good.

Any ideas on how to fix it?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Frederic Weisbecker @ 2010-01-26  2:35 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <4B5E54C0.2040709@kernel.org>

On Tue, Jan 26, 2010 at 11:34:40AM +0900, Tejun Heo wrote:
> On 01/26/2010 11:22 AM, Frederic Weisbecker wrote:
> >> What about this? It doesn't use direct cast to scalar but should
> >> create a pointer type to kernel space datas:
> >>
> >>
> >> #define kernel_space_t(var)	\
> >> 	(typeof(var)	__kernel __force)
> > 
> > 
> > Should be typeof(*var)
> > 
> >>
> >> #define SHIFT_PERCPU_PTR(__p, __offset) ({                              \
> >>         __verify_pcpu_ptr((__p));                                       \
> >>         RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
> >> })
> 
> Can you balance ()'s here too?


#define kernel_space_t(var)        \
    (typeof(*(var))    __kernel __force)


/* Weird cast keeps both GCC and sparse happy. */
#define SHIFT_PERCPU_PTR(__p, __offset) ({                              \
        __verify_pcpu_ptr((__p));                                       \
        RELOC_HIDE((typeof(*(kernel_space_t(__p))) __kernel __force *)(__p), (__offset)); \
})

#define per_cpu(var, cpu) \
        (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)));


No guarantee that will build. I should pull your tree and install
sparse (yeah, shame on me, I've never installed it).

^ permalink raw reply

* Re: [PATCH v2]IP: Send an ICMP "Fragment Reassembly Timeout" message when enabling connection track
From: Yasuyuki KOZAKAI @ 2010-01-26  2:34 UTC (permalink / raw)
  To: shanwei
  Cc: yasuyuki.kozakai, kaber, davem, kuznet, pekkas, jmorris, yoshfuji,
	eric.dumazet, david, jorge, opurdila, netdev, netfilter-devel
In-Reply-To: <4B5E44A2.2080006@cn.fujitsu.com>


Hi,

From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Tue, 26 Jan 2010 09:25:54 +0800

> Yasuyuki KOZAKAI wrote, at 01/25/2010 08:57 AM:
> > It sounds good. Please take care that IPv6 router does not reassemble
> > fragmented packets. 
> 
> I don't know the details about IPv6 router implement.
> Did you mean that we can not directly use ip6_route_input(skb) to find
> Routing type(host/router)? 

I just talked about RFC2460.

4.5  Fragment Header

   The Fragment header is used by an IPv6 source to send a packet larger
   than would fit in the path MTU to its destination.  (Note: unlike
   IPv4, fragmentation in IPv6 is performed only by source nodes, not by
   routers along a packet's delivery path -- see section 5.) 


> > IIRC the current nf_conntrack_{ipv6,reasm}.c
> > reassembles the cloned skbs for tracking, discard the cloned skbs after
> > tracking and forward the original skbs to IPv6 stack to keep the size of
> > fragmented packets.
> 
> Indeed, after assembling fragments successfully in IPv6 connection track,
> original fragments are forwarded to IPv6 stack. And then IPv6 stack
> also assembles those received fragments again.
> Thus fragments are assembled twice. 

Yes, in the case that Linux is IPv6 host.


> But IPv4 only assembly once. IPv4 connection track assembles fragments
> > successfully and then just forwards assembled intact packet to IPv4
> > stack. 
> Do you know why is IPv6 designed like that?

General speaking, IPv6 router just forwards the fragmented packets and
it's up to IPv6 host to handle them. And nf_conntrack is not packet filter,
but it just tracks packets. So I designed that nf_contrack_ipv6 forwards
fragments to IPv6 stack even if nf_conntrack detects missing piece
of fragments. This resulted in twice reassembly in the case that
Linux is IPv6 host, but I tolerated that.

I think that your improvement can remove such inefficient processing.

BTW, I explained the reason why nf_conntrack does not re-fragment
the reassembled skb like nf_conntrack_ipv4 and forwards the original
fragments to IPv6 stack in

	http://conferences.sigcomm.org/sigcomm/2007/ipv6/1569042997.pdf

Regards,

-- Yasuyuki Kozakai

^ permalink raw reply

* Re: [PATCH] starfire: Clean up properly if firmware loading fails
From: Ben Hutchings @ 2010-01-26  2:32 UTC (permalink / raw)
  To: Andrew Morton
  Cc: netdev, bugzilla-daemon, bugme-daemon, michael, Alan Cox, stable,
	David S. Miller
In-Reply-To: <20100125181508.790010cb.akpm@linux-foundation.org>

On Mon, 2010-01-25 at 18:15 -0800, Andrew Morton wrote:
> On Tue, 26 Jan 2010 02:02:12 +0000 Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > netdev_open() will return without cleaning up net device or hardware
> > state if firmware loading fails.  This results in a BUG() on a second
> > attempt to bring the interface up, reported in
> > <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
> > even worse effects if the driver is removed afterwards.
> 
> (That's the wrong bugzilla URL)

Hah, well spotted.  Must have copied from the wrong tab.

> > Call netdev_close() to clean up on failure.
> 
> OK, thanks.
> 
> > ---
> > On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> > > (switched to email.  Please respond via emailed reply-to-all, not via the
> > > bugzilla web interface).
> > > 
> > > On Wed, 20 Jan 2010 04:29:20 GMT
> > > bugzilla-daemon@bugzilla.kernel.org wrote:
> > > 
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > > > 
> > > >            Summary: starfire causes kernel BUG when interface goes up
> > [...] 
> > > > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > > > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > > > ethX up" (where X is a starfire port).
> > [...] 
> > > Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> > > &n->state)); in napi_enable().
> > > 
> > > This is a regression somewhere between 2.6.24 and 2.6.32(!).
> > 
> > This driver now attempts to load firmware when an interface is brought
> > up, *after* calling napi_enable().  If that fails, it will return
> > without calling napi_disable().  On the second attempt to bring the
> > interface it calls napi_enable() a second time and triggers this
> > assertion.
> > 
> > As a workaround, try installing the necessary firmware. :-)
> > 
> 
> Missing signed-off-by.  I added it, OK?  Also added a Cc:stable.
[...]

Well I was going to wait and see what the submitter says first, but I'm
happy for you to add that in your patch set if it's a positive result.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Al Viro @ 2010-01-26  2:32 UTC (permalink / raw)
  To: Tejun Heo
  Cc: H. Peter Anvin, Frederic Weisbecker, linux-kernel, axboe, rusty,
	akpm, ebiederm, tytso, Trond.Myklebust, aelder, hch, davem,
	netdev, x86, mingo, dan.j.williams, borislav.petkov, ying.huang,
	lenb, neilb, cl
In-Reply-To: <4B5E508A.1030305@kernel.org>

On Tue, Jan 26, 2010 at 11:16:42AM +0900, Tejun Heo wrote:

> # define RELOC_HIDE(ptr, off)					\
>   ({ unsigned long __ptr;					\
>      __ptr = (unsigned long) (ptr);				\
>     (typeof(ptr)) (__ptr + (off)); })
> 
> #define __verify_pcpu_ptr(ptr)	do {					\
> 	const void __percpu *__vpp_verify = (typeof(ptr))NULL;		\
> 	(void)__vpp_verify;						\
> } while (0)
> 
> /* Weird cast keeps both GCC and sparse happy. */
> #define SHIFT_PERCPU_PTR(__p, __offset)	({				\
> 	__verify_pcpu_ptr((__p));					\
> 	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
> })
> 
> #define per_cpu(var, cpu) \
> 	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

Eh...  You are leaving that noderef in place in case of array.  And _that_
is not an address space, so casts to AS 0 won't do you any good.

^ permalink raw reply

* [PATCH 2/2] IPv6: conntrack: Use protocol-related match routine in IPv6 connection track
From: Shan Wei @ 2010-01-26  2:31 UTC (permalink / raw)
  To: Patrick McHardy, David Miller, Yasuyuki KOZAKAI
  Cc: netfilter-devel, netdev@vger.kernel.org


IPv6 connection track and IPv6 stack separately use a different queue to 
manage received fragments. So using protocol-related match routine.


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 include/net/ipv6.h                      |    1 -
 net/ipv6/netfilter/nf_conntrack_reasm.c |   13 ++++++++++++-
 net/ipv6/reassembly.c                   |    3 +--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index ccab594..cbd768b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -365,7 +365,6 @@ struct ip6_create_arg {
 };
 
 void ip6_frag_init(struct inet_frag_queue *q, void *a);
-int ip6_frag_match(struct inet_frag_queue *q, void *a);
 
 static inline int ipv6_addr_any(const struct in6_addr *a)
 {
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 312c20a..66b6161 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -146,6 +146,17 @@ static void nf_ct_frag6_evictor(void)
 	local_bh_enable();
 }
 
+static int nf_ct_frag_match(struct inet_frag_queue *q, void *a)
+{
+	struct nf_ct_frag6_queue *fq;
+	struct ip6_create_arg *arg = a;
+	fq = container_of(q, struct nf_ct_frag6_queue, q);
+
+	return (fq->id == arg->id &&
+		ipv6_addr_equal(&fq->saddr, arg->src) &&
+		ipv6_addr_equal(&fq->daddr, arg->dst));
+}
+
 static void nf_ct_frag6_expire(unsigned long data)
 {
 	struct nf_ct_frag6_queue *fq;
@@ -665,7 +676,7 @@ int nf_ct_frag6_init(void)
 	nf_frags.destructor = NULL;
 	nf_frags.skb_free = nf_skb_free;
 	nf_frags.qsize = sizeof(struct nf_ct_frag6_queue);
-	nf_frags.match = ip6_frag_match;
+	nf_frags.match = nf_ct_frag_match;
 	nf_frags.frag_expire = nf_ct_frag6_expire;
 	nf_frags.secret_interval = 10 * 60 * HZ;
 	nf_init_frags.timeout = IPV6_FRAG_TIMEOUT;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 2cddea3..2fa4355 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -136,7 +136,7 @@ static unsigned int ip6_hashfn(struct inet_frag_queue *q)
 	return inet6_hash_frag(fq->id, &fq->saddr, &fq->daddr, ip6_frags.rnd);
 }
 
-int ip6_frag_match(struct inet_frag_queue *q, void *a)
+static int ip6_frag_match(struct inet_frag_queue *q, void *a)
 {
 	struct frag_queue *fq;
 	struct ip6_create_arg *arg = a;
@@ -146,7 +146,6 @@ int ip6_frag_match(struct inet_frag_queue *q, void *a)
 			ipv6_addr_equal(&fq->saddr, arg->src) &&
 			ipv6_addr_equal(&fq->daddr, arg->dst));
 }
-EXPORT_SYMBOL(ip6_frag_match);
 
 /* Memory Tracking Functions. */
 static inline void frag_kfree_skb(struct netns_frags *nf,
-- 
1.6.3.3



^ permalink raw reply related

* [PATCH 1/2] IPv6: conntrack: Use protocol-related initialization routine to initial queues of IPv6 connection track
From: Shan Wei @ 2010-01-26  2:31 UTC (permalink / raw)
  To: Patrick McHardy, David Miller, Yasuyuki KOZAKAI
  Cc: netfilter-devel, netdev@vger.kernel.org


IPv6 connection track and IPv6 stack separately use a different queue to 
manage received fragments. The former uses nf_ct_frag6_queue structure, 
the latter uses frag_queue structure.

When creating new queue for IPv6 connection track, ip6_frag_init() 
that belongs to IPv6 stack is called to initial nf_ct_frag6_queue structure. 
This broken the saddr&daddr member in nf_ct_frag6_queue, and then hash value 
generated by nf_hashfn() is not equal with that generated by fq_find(). 
So, a new received fragment can't be inserted to right queue.
 
The patch fixes the bug with protocol-related initialization routine.
The patch-set have been tested.


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 include/net/ipv6.h                      |    1 -
 net/ipv6/netfilter/nf_conntrack_reasm.c |   13 ++++++++++++-
 net/ipv6/reassembly.c                   |    3 +--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index cbd768b..a7112da 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -364,7 +364,6 @@ struct ip6_create_arg {
 	struct in6_addr *dst;
 };
 
-void ip6_frag_init(struct inet_frag_queue *q, void *a);
 
 static inline int ipv6_addr_any(const struct in6_addr *a)
 {
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 66b6161..4a61d14 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -146,6 +146,17 @@ static void nf_ct_frag6_evictor(void)
 	local_bh_enable();
 }
 
+static void nf_ct_queue_init(struct inet_frag_queue *q, void *a)
+{
+	struct nf_ct_frag6_queue *fq;
+	struct ip6_create_arg *arg = a;
+
+	fq = container_of(q, struct nf_ct_frag6_queue, q);
+	fq->id = arg->id;
+	ipv6_addr_copy(&fq->saddr, arg->src);
+	ipv6_addr_copy(&fq->daddr, arg->dst);
+}
+
 static int nf_ct_frag_match(struct inet_frag_queue *q, void *a)
 {
 	struct nf_ct_frag6_queue *fq;
@@ -672,7 +683,7 @@ void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
 int nf_ct_frag6_init(void)
 {
 	nf_frags.hashfn = nf_hashfn;
-	nf_frags.constructor = ip6_frag_init;
+	nf_frags.constructor = nf_ct_frag_init;
 	nf_frags.destructor = NULL;
 	nf_frags.skb_free = nf_skb_free;
 	nf_frags.qsize = sizeof(struct nf_ct_frag6_queue);
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 2fa4355..9f9b6a2 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -157,7 +157,7 @@ static inline void frag_kfree_skb(struct netns_frags *nf,
 	kfree_skb(skb);
 }
 
-void ip6_frag_init(struct inet_frag_queue *q, void *a)
+static void ip6_frag_init(struct inet_frag_queue *q, void *a)
 {
 	struct frag_queue *fq = container_of(q, struct frag_queue, q);
 	struct ip6_create_arg *arg = a;
@@ -167,7 +167,6 @@ void ip6_frag_init(struct inet_frag_queue *q, void *a)
 	ipv6_addr_copy(&fq->saddr, arg->src);
 	ipv6_addr_copy(&fq->daddr, arg->dst);
 }
-EXPORT_SYMBOL(ip6_frag_init);
 
 /* Destruction primitives. */
 
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:34 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <20100126022211.GP5087@nowhere>

On 01/26/2010 11:22 AM, Frederic Weisbecker wrote:
>> What about this? It doesn't use direct cast to scalar but should
>> create a pointer type to kernel space datas:
>>
>>
>> #define kernel_space_t(var)	\
>> 	(typeof(var)	__kernel __force)
> 
> 
> Should be typeof(*var)
> 
>>
>> #define SHIFT_PERCPU_PTR(__p, __offset) ({                              \
>>         __verify_pcpu_ptr((__p));                                       \
>>         RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
>> })

Can you balance ()'s here too?

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Frederic Weisbecker @ 2010-01-26  2:22 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <20100126021846.GO5087@nowhere>

On Tue, Jan 26, 2010 at 03:18:48AM +0100, Frederic Weisbecker wrote:
> On Tue, Jan 26, 2010 at 11:13:59AM +0900, Tejun Heo wrote:
> > Hello,
> > 
> > On 01/26/2010 11:01 AM, Frederic Weisbecker wrote:
> > > So, may be it considers you are applying the address space overriding
> > > to the pointer to the type and not to the type itself.
> > > 
> > > Consider:
> > > 
> > > 	int __percpu i;
> > > 
> > > What you do above *might* be considered as if SHIFT_PERCPU_PTR
> > > returns something of a type:
> > > 
> > > 	int * __percpu i;
> > > 
> > > So the pointer is in the normal address space, but its content is in
> > > __percpu address space.
> > > 
> > > What if you do this:
> > > 
> > > 
> > > #define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
> > >       __verify_pcpu_ptr((__p));                                       \
> > >       RELOC_HIDE((__p), (__offset)); \
> > > })
> > > 
> > > #define per_cpu(var, cpu) \
> > >       (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> > 
> > arch/x86/kernel/cpu/common.c:1149:20: warning: cast to non-scalar
> > arch/x86/kernel/cpu/common.c:1149:20: error: strange non-value function or array
> >   CC      arch/x86/kernel/cpu/common.o
> > arch/x86/kernel/cpu/common.c: In function 'cpu_init':
> > arch/x86/kernel/cpu/common.c:1149: error: cast specifies array type
> > 
> > Can't cast that way.  :-(
> 
> 
> What about this? It doesn't use direct cast to scalar but should
> create a pointer type to kernel space datas:
> 
> 
> #define kernel_space_t(var)	\
> 	(typeof(var)	__kernel __force)


Should be typeof(*var)



> 
> #define SHIFT_PERCPU_PTR(__p, __offset) ({                              \
>         __verify_pcpu_ptr((__p));                                       \
>         RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
> })
> 
> #define per_cpu(var, cpu) \
>         (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 


^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Frederic Weisbecker @ 2010-01-26  2:18 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <4B5E4FE7.30700@kernel.org>

On Tue, Jan 26, 2010 at 11:13:59AM +0900, Tejun Heo wrote:
> Hello,
> 
> On 01/26/2010 11:01 AM, Frederic Weisbecker wrote:
> > So, may be it considers you are applying the address space overriding
> > to the pointer to the type and not to the type itself.
> > 
> > Consider:
> > 
> > 	int __percpu i;
> > 
> > What you do above *might* be considered as if SHIFT_PERCPU_PTR
> > returns something of a type:
> > 
> > 	int * __percpu i;
> > 
> > So the pointer is in the normal address space, but its content is in
> > __percpu address space.
> > 
> > What if you do this:
> > 
> > 
> > #define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
> >       __verify_pcpu_ptr((__p));                                       \
> >       RELOC_HIDE((__p), (__offset)); \
> > })
> > 
> > #define per_cpu(var, cpu) \
> >       (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 
> arch/x86/kernel/cpu/common.c:1149:20: warning: cast to non-scalar
> arch/x86/kernel/cpu/common.c:1149:20: error: strange non-value function or array
>   CC      arch/x86/kernel/cpu/common.o
> arch/x86/kernel/cpu/common.c: In function 'cpu_init':
> arch/x86/kernel/cpu/common.c:1149: error: cast specifies array type
> 
> Can't cast that way.  :-(


What about this? It doesn't use direct cast to scalar but should
create a pointer type to kernel space datas:


#define kernel_space_t(var)	\
	(typeof(var)	__kernel __force)

#define SHIFT_PERCPU_PTR(__p, __offset) ({                              \
        __verify_pcpu_ptr((__p));                                       \
        RELOC_HIDE((typeof(*(kernel_space_t(var)) __kernel __force *)(__p), (__offset)); \
})

#define per_cpu(var, cpu) \
        (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

^ permalink raw reply

* Re: [PATCH 5/8] percpu: add __percpu sparse annotations to x86
From: Tejun Heo @ 2010-01-26  2:17 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	fweisbec, dan.j.williams, borislav.petkov, ying.huang, lenb,
	neilb, cl, Thomas Gleixner
In-Reply-To: <4B5E401C.1050605@zytor.com>

On 01/26/2010 10:06 AM, H. Peter Anvin wrote:
> On 01/25/2010 07:22 AM, Tejun Heo wrote:
>>
>> In arch/x86/kernel/cpu/common.c, per_cpu(exception_stacks, cpu) is
>> replaced with &per_cpu(exception_stacks[0], cpu) which is equivalent
>> except that it allows per_cpu() macro to correctly drop percpu
>> designation during sparse pass.
>>
> 
> Same comment as Frederic's about this... this kind of uglification
> really is unacceptable and needs to be addressed.

Hmm... I don't think it's too bad but if you don't like it that much,
probably the right thing to do is just leave it there and let sparse
whine about it.  There are only three in-kernel instances at this
point, so it's not that big a deal.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] starfire: Clean up properly if firmware loading fails
From: Andrew Morton @ 2010-01-26  2:15 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, bugzilla-daemon, bugme-daemon, michael, Alan Cox, stable,
	David S. Miller
In-Reply-To: <1264471333.373.349.camel@localhost>

On Tue, 26 Jan 2010 02:02:12 +0000 Ben Hutchings <bhutchings@solarflare.com> wrote:

> netdev_open() will return without cleaning up net device or hardware
> state if firmware loading fails.  This results in a BUG() on a second
> attempt to bring the interface up, reported in
> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
> even worse effects if the driver is removed afterwards.

(That's the wrong bugzilla URL)

> 
> Call netdev_close() to clean up on failure.

OK, thanks.

> ---
> On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> > (switched to email.  Please respond via emailed reply-to-all, not via the
> > bugzilla web interface).
> > 
> > On Wed, 20 Jan 2010 04:29:20 GMT
> > bugzilla-daemon@bugzilla.kernel.org wrote:
> > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > > 
> > >            Summary: starfire causes kernel BUG when interface goes up
> [...] 
> > > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > > ethX up" (where X is a starfire port).
> [...] 
> > Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> > &n->state)); in napi_enable().
> > 
> > This is a regression somewhere between 2.6.24 and 2.6.32(!).
> 
> This driver now attempts to load firmware when an interface is brought
> up, *after* calling napi_enable().  If that fails, it will return
> without calling napi_disable().  On the second attempt to bring the
> interface it calls napi_enable() a second time and triggers this
> assertion.
> 
> As a workaround, try installing the necessary firmware. :-)
> 

Missing signed-off-by.  I added it, OK?  Also added a Cc:stable.


From: Ben Hutchings <bhutchings@solarflare.com>

netdev_open() will return without cleaning up net device or hardware state
if firmware loading fails.  This results in a BUG() on a second attempt to
bring the interface up, reported in
<http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
worse effects if the driver is removed afterwards.

Call netdev_close() to clean up on failure.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=15091

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Reported-by: Michael Moffatt <michael@moffatt.org.nz>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/starfire.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff -puN drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails drivers/net/starfire.c
--- a/drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails
+++ a/drivers/net/starfire.c
@@ -1063,7 +1063,7 @@ static int netdev_open(struct net_device
 	if (retval) {
 		printk(KERN_ERR "starfire: Failed to load firmware \"%s\"\n",
 		       FIRMWARE_RX);
-		return retval;
+		goto out_init;
 	}
 	if (fw_rx->size % 4) {
 		printk(KERN_ERR "starfire: bogus length %zu in \"%s\"\n",
@@ -1108,6 +1108,9 @@ out_tx:
 	release_firmware(fw_tx);
 out_rx:
 	release_firmware(fw_rx);
+out_init:
+	if (retval)
+		netdev_close(dev);
 	return retval;
 }
 
_




^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:13 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <20100126020113.GM5087@nowhere>

Hello,

On 01/26/2010 11:01 AM, Frederic Weisbecker wrote:
> So, may be it considers you are applying the address space overriding
> to the pointer to the type and not to the type itself.
> 
> Consider:
> 
> 	int __percpu i;
> 
> What you do above *might* be considered as if SHIFT_PERCPU_PTR
> returns something of a type:
> 
> 	int * __percpu i;
> 
> So the pointer is in the normal address space, but its content is in
> __percpu address space.
> 
> What if you do this:
> 
> 
> #define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
>       __verify_pcpu_ptr((__p));                                       \
>       RELOC_HIDE((__p), (__offset)); \
> })
> 
> #define per_cpu(var, cpu) \
>       (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

arch/x86/kernel/cpu/common.c:1149:20: warning: cast to non-scalar
arch/x86/kernel/cpu/common.c:1149:20: error: strange non-value function or array
  CC      arch/x86/kernel/cpu/common.o
arch/x86/kernel/cpu/common.c: In function 'cpu_init':
arch/x86/kernel/cpu/common.c:1149: error: cast specifies array type

Can't cast that way.  :-(

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:16 UTC (permalink / raw)
  To: Al Viro
  Cc: H. Peter Anvin, Frederic Weisbecker, linux-kernel, axboe, rusty,
	akpm, ebiederm, tytso, Trond.Myklebust, aelder, hch, davem,
	netdev, x86, mingo, dan.j.williams, borislav.petkov, ying.huang,
	lenb, neilb, cl
In-Reply-To: <20100126020452.GB19799@ZenIV.linux.org.uk>

On 01/26/2010 11:04 AM, Al Viro wrote:
> On Tue, Jan 26, 2010 at 11:06:10AM +0900, Tejun Heo wrote:
>> Hello,
>>
>> On 01/26/2010 10:02 AM, H. Peter Anvin wrote:
>>> A cast (using __typeof__) combined with an address space override?
>>
>> That still puts the address specification at the wrong level.  The
>> problem is that the __typeof__ would be an array type which itself
>> doesn't have address space set but its members are in address space 3.
>> So, you need to get *inside* the array type def to change that.  :-(
> 
> Could you post the actual definitions in one piece?  Would be easier to
> discuss what's going on...

Here it is.

# define RELOC_HIDE(ptr, off)					\
  ({ unsigned long __ptr;					\
     __ptr = (unsigned long) (ptr);				\
    (typeof(ptr)) (__ptr + (off)); })

#define __verify_pcpu_ptr(ptr)	do {					\
	const void __percpu *__vpp_verify = (typeof(ptr))NULL;		\
	(void)__vpp_verify;						\
} while (0)

/* Weird cast keeps both GCC and sparse happy. */
#define SHIFT_PERCPU_PTR(__p, __offset)	({				\
	__verify_pcpu_ptr((__p));					\
	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
})

#define per_cpu(var, cpu) \
	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:10 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Frederic Weisbecker, linux-kernel, axboe, rusty, akpm, ebiederm,
	tytso, Trond.Myklebust, aelder, hch, viro, davem, netdev, x86,
	mingo, dan.j.williams, borislav.petkov, ying.huang, lenb, neilb,
	cl
In-Reply-To: <4B5E401D.2020809@zytor.com>

Hello,

On 01/26/2010 10:06 AM, H. Peter Anvin wrote:
>> I guess __percpu is an address_space attribute? Is there no
>> way to force the address space change directly from the
>> per_cpu() macro?
>>
> 
> Hmm... thinking more about it, we should be able to just move the & and
> [0] into the per_cpu() macro, addressing the situation, or does that
> cause problems elsewhere?

Hmmm.... the argument gotta be an lvalue.  Let me try just in case.
Nope, it doesn't work.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Frederic Weisbecker @ 2010-01-26  2:10 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <20100126020113.GM5087@nowhere>

On Tue, Jan 26, 2010 at 03:01:14AM +0100, Frederic Weisbecker wrote:
> On Tue, Jan 26, 2010 at 10:19:04AM +0900, Tejun Heo wrote:
> > Hello,
> > 
> > On 01/26/2010 10:02 AM, Frederic Weisbecker wrote:
> > > Well, sorry I must be missing something obvious, but is it impossible
> > > to make per_cpu(var, cpu) returning something cast in:
> > > 
> > > 	(typeof(var) __force)
> > > 
> > > Or I guess you did that already and it is not working with static
> > > arrays, or?
> > 
> > Yeap, the definition looks like
> > 
> >  #define SHIFT_PERCPU_PTR(__p, __offset)	({			\
> > 	__verify_pcpu_ptr((__p));					\
> > 	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
> >  })
> > 
> >  #define per_cpu(var, cpu) \
> > 	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> > 
> > but it just ends up putting the __force at the wrong layer.  It seems
> > that (typeof(var) __kernel __force) tell sparse var is in the kernel
> > address space but not its members.
> 
> 
> So, may be it considers you are applying the address space overriding
> to the pointer to the type and not to the type itself.
> 
> Consider:
> 
> 	int __percpu i;
> 
> What you do above *might* be considered as if SHIFT_PERCPU_PTR
> returns something of a type:
> 
> 	int * __percpu i;
> 
> So the pointer is in the normal address space, but its content is in
> __percpu address space.
> 
> What if you do this:
> 
> 
> #define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
>       __verify_pcpu_ptr((__p));                                       \
>       RELOC_HIDE((__p), (__offset)); \
> })
> 
> #define per_cpu(var, cpu) \
>       (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 
> This should work because &(var) should be dereferencable directly, since
> it is not of type "__force t" but of type "*__force t"
> 
> And you're not doing anymore this:
> 
> 	*(int * __kernel __force) i;
> but
> 	*(int __kernel __force *) i;



The above is perhaps a bit confusing.
To be more clear, in the first case you only cast the pointer
to the type, which gives you a pointer valid in kernel space
to data valid in percpu space.

The second case gives you something valid in kernel space for both.

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Tejun Heo @ 2010-01-26  2:06 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Frederic Weisbecker, linux-kernel, axboe, rusty, akpm, ebiederm,
	tytso, Trond.Myklebust, aelder, hch, viro, davem, netdev, x86,
	mingo, dan.j.williams, borislav.petkov, ying.huang, lenb, neilb,
	cl
In-Reply-To: <4B5E3F2C.2010702@zytor.com>

Hello,

On 01/26/2010 10:02 AM, H. Peter Anvin wrote:
> A cast (using __typeof__) combined with an address space override?

That still puts the address specification at the wrong level.  The
problem is that the __typeof__ would be an array type which itself
doesn't have address space set but its members are in address space 3.
So, you need to get *inside* the array type def to change that.  :-(

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Al Viro @ 2010-01-26  2:04 UTC (permalink / raw)
  To: Tejun Heo
  Cc: H. Peter Anvin, Frederic Weisbecker, linux-kernel, axboe, rusty,
	akpm, ebiederm, tytso, Trond.Myklebust, aelder, hch, davem,
	netdev, x86, mingo, dan.j.williams, borislav.petkov, ying.huang,
	lenb, neilb, cl
In-Reply-To: <4B5E4E12.7060900@kernel.org>

On Tue, Jan 26, 2010 at 11:06:10AM +0900, Tejun Heo wrote:
> Hello,
> 
> On 01/26/2010 10:02 AM, H. Peter Anvin wrote:
> > A cast (using __typeof__) combined with an address space override?
> 
> That still puts the address specification at the wrong level.  The
> problem is that the __typeof__ would be an array type which itself
> doesn't have address space set but its members are in address space 3.
> So, you need to get *inside* the array type def to change that.  :-(

Could you post the actual definitions in one piece?  Would be easier to
discuss what's going on...

^ permalink raw reply

* RE: Bug#565404: linux-image-2.6.26-2-amd64: atl1e: TSO is broken
From: Jie Yang @ 2010-01-26  2:04 UTC (permalink / raw)
  To: Anders Boström
  Cc: ben@decadent.org.uk, netdev@vger.kernel.org,
	565404@bugs.debian.org, Xiong Huang
In-Reply-To: <20100125.163612.582489483209420341.anders@netinsight.net>

Anders Boström <anders@netinsight.net> wrote:
>> IP-header length field, but is shorter.
>  >>
>  JY> following is my test cese,
>
>  JY> a nfs server server with ar8131chip, device id 1063.
> export /tmp/ dir as the nfs share directory,  JY> the client,
> mount the server_ip:/tmp to local dir /mnt/nfs, ust a python
> script to write and read data on the  JY>
> /mnt/nfs/testnfs.log. it works fine.
>
> OK, the device-ID in our NFS-server is 1026, rev. b0. So it
> is possible that the problem is specific to that chip/version.
oops, its my mistake in writing, my case is 1026 device ID

>
>  JY> Can you give me some advice on how to reproduce this bug??
>
> The only suggestion I have is to try to find a board with a
> 1026-chip on it.
>
> My test-case is just copy of a 1 Gbyte file from the
> NFS-server to /dev/null , after making sure that the file
> isn't cached on the client by reading huge amounts of other data.
>
just to check, if the kernel version is 2.6.26-2 ??

Best wishes
jie

^ permalink raw reply

* Re: Netlink usage question (for bonding comm with userspace)
From: David Miller @ 2010-01-26  2:03 UTC (permalink / raw)
  To: fubar; +Cc: netdev
In-Reply-To: <19462.1264468440@death.nxdomain.ibm.com>

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Mon, 25 Jan 2010 17:14:00 -0800

> 	Just so I'm clear, though: use generic netlink for the needs of
> this particular project, and then later it's acceptable to have both
> generic and rtnetlink co-existing for their respective uses?

Use generic netlink for everything.

It's just a different namespace ("text strings", vs. fixed integer
IDs) for netlink sockets, nothing more really.

^ permalink raw reply

* [PATCH] starfire: Clean up properly if firmware loading fails
From: Ben Hutchings @ 2010-01-26  2:02 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, bugzilla-daemon, bugme-daemon, michael, Alan Cox
In-Reply-To: <20100125170816.db9435ed.akpm@linux-foundation.org>

netdev_open() will return without cleaning up net device or hardware
state if firmware loading fails.  This results in a BUG() on a second
attempt to bring the interface up, reported in
<http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
even worse effects if the driver is removed afterwards.

Call netdev_close() to clean up on failure.
---
On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Wed, 20 Jan 2010 04:29:20 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > 
> >            Summary: starfire causes kernel BUG when interface goes up
[...] 
> > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > ethX up" (where X is a starfire port).
[...] 
> Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> &n->state)); in napi_enable().
> 
> This is a regression somewhere between 2.6.24 and 2.6.32(!).

This driver now attempts to load firmware when an interface is brought
up, *after* calling napi_enable().  If that fails, it will return
without calling napi_disable().  On the second attempt to bring the
interface it calls napi_enable() a second time and triggers this
assertion.

As a workaround, try installing the necessary firmware. :-)

Ben.

 drivers/net/starfire.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
index 95db60a..f952113 100644
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -1063,7 +1063,7 @@ static int netdev_open(struct net_device *dev)
 	if (retval) {
 		printk(KERN_ERR "starfire: Failed to load firmware \"%s\"\n",
 		       FIRMWARE_RX);
-		return retval;
+		goto out_init;
 	}
 	if (fw_rx->size % 4) {
 		printk(KERN_ERR "starfire: bogus length %zu in \"%s\"\n",
@@ -1108,6 +1108,9 @@ out_tx:
 	release_firmware(fw_tx);
 out_rx:
 	release_firmware(fw_rx);
+out_init:
+	if (retval)
+		netdev_close(dev);
 	return retval;
 }
 
-- 
1.6.6

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint
From: Frederic Weisbecker @ 2010-01-26  2:01 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, axboe, rusty, akpm, ebiederm, tytso,
	Trond.Myklebust, aelder, hch, viro, davem, netdev, x86, mingo,
	dan.j.williams, borislav.petkov, ying.huang, lenb, neilb, cl
In-Reply-To: <4B5E4308.8020105@kernel.org>

On Tue, Jan 26, 2010 at 10:19:04AM +0900, Tejun Heo wrote:
> Hello,
> 
> On 01/26/2010 10:02 AM, Frederic Weisbecker wrote:
> > Well, sorry I must be missing something obvious, but is it impossible
> > to make per_cpu(var, cpu) returning something cast in:
> > 
> > 	(typeof(var) __force)
> > 
> > Or I guess you did that already and it is not working with static
> > arrays, or?
> 
> Yeap, the definition looks like
> 
>  #define SHIFT_PERCPU_PTR(__p, __offset)	({			\
> 	__verify_pcpu_ptr((__p));					\
> 	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
>  })
> 
>  #define per_cpu(var, cpu) \
> 	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 
> but it just ends up putting the __force at the wrong layer.  It seems
> that (typeof(var) __kernel __force) tell sparse var is in the kernel
> address space but not its members.


So, may be it considers you are applying the address space overriding
to the pointer to the type and not to the type itself.

Consider:

	int __percpu i;

What you do above *might* be considered as if SHIFT_PERCPU_PTR
returns something of a type:

	int * __percpu i;

So the pointer is in the normal address space, but its content is in
__percpu address space.

What if you do this:


#define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
      __verify_pcpu_ptr((__p));                                       \
      RELOC_HIDE((__p), (__offset)); \
})

#define per_cpu(var, cpu) \
      (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

This should work because &(var) should be dereferencable directly, since
it is not of type "__force t" but of type "*__force t"

And you're not doing anymore this:

	*(int * __kernel __force) i;
but
	*(int __kernel __force *) i;


That all might make no sense, I'm just trying to think like a backend
so it might sound like I should just take more sleep and just shut up...

^ permalink raw reply

* Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up
From: Michael @ 2010-01-26  1:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, bugzilla-daemon, bugme-daemon, Alan Cox
In-Reply-To: <20100125170816.db9435ed.akpm@linux-foundation.org>

Hi Andrew,

I believe that this is a regression, yes.

I will attempt to compile up some kernels this week and provide more 
info. Should I start at 26 and go up or at 31 and go down?

I can't use anything lower than 26 according to udev. I was running 24 
but compiled 32 when I upgraded udev.

Regards,
Michael.

Andrew Morton wrote:
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Wed, 20 Jan 2010 04:29:20 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>   
>> http://bugzilla.kernel.org/show_bug.cgi?id=15091
>>
>>            Summary: starfire causes kernel BUG when interface goes up
>>            Product: Drivers
>>            Version: 2.5
>>     Kernel Version: 2.6.32
>>           Platform: All
>>         OS/Version: Linux
>>               Tree: Mainline
>>             Status: NEW
>>           Severity: normal
>>           Priority: P1
>>          Component: Network
>>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>>         ReportedBy: michael@moffatt.org.nz
>>         Regression: No
>>
>>
>> Created an attachment (id=24651)
>>  --> (http://bugzilla.kernel.org/attachment.cgi?id=24651)
>> ls -l /dev (before crash)
>>
>> I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
>> cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
>> ethX up" (where X is a starfire port).
>>
>> Sometimes the exception causes the whole kernel to freeze. Sometimes the kernel
>> keeps going. On the occasion that the kernel kept going I was able to retrieve
>> syslog, which has the full kernel information.
>>
>> Note that in syslog, you can see that I inserted a USB memory stick in order to
>> copy off the attached files. The kernel oops happens without the USB memory
>> stick inserted.
>>
>> I can reproduce this at will. At the moment I simply can't use my two four port
>> starfire network cards.
>>
>> This PC is a root-over-NFS system.
>>
>>     
>
> Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> &n->state)); in napi_enable().
>
> This is a regression somewhere between 2.6.24 and 2.6.32(!).
>
>
>   


^ permalink raw reply

* Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up
From: Andrew Morton @ 2010-01-26  1:51 UTC (permalink / raw)
  To: Michael; +Cc: netdev, bugzilla-daemon, bugme-daemon, Alan Cox
In-Reply-To: <4B5E48FF.2050406@moffatt.org.nz>

On Tue, 26 Jan 2010 14:44:31 +1300 Michael <michael@moffatt.org.nz> wrote:

> Hi Andrew,
> 
> I believe that this is a regression, yes.
> 
> I will attempt to compile up some kernels this week and provide more 
> info. Should I start at 26 and go up or at 31 and go down?
> 
> I can't use anything lower than 26 according to udev. I was running 24 
> but compiled 32 when I upgraded udev.
> 

Thanks.

Starfire is a pretty rarely-used driver, I suspect.  Hopefully someone
who understands the NAPI stuff can look at the code and go "ah-hah",
and save you all that work.

But if that doesn't happen then yup, a bisection would be good, thanks.
 The best way to do it really is with git. 
http://landley.net/writing/git-quick.html has an explanation.




^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox