| Message ID | 1341555227.3265.54.camel@edumazet-glaptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2012-07-06 at 08:13 +0200, Eric Dumazet wrote: > On Fri, 2012-07-06 at 13:58 +0800, Fengguang Wu wrote: > > On Thu, Jul 05, 2012 at 02:22:00PM -0700, David Miller wrote: > > > > > > Steffen Klassert posted a patch which fixes this. > > > > Steffen's patch converts one oops message into another. > > > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master > > head: 700db99d0140e9da2a31e08ebd3e1b121691aa26 > > commit: a2de86f63cfc92f7aaf11e7b9d9f2150946a1622 [1/2] ipv6: Initialize the neighbour pointer of rt6_info on allocation > > > > x86_64-allyesdebian BBB > > Please try : > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index 6d9c0ab..c6af596 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -975,7 +975,7 @@ static int ip6_dst_lookup_tail(struct sock *sk, > * dst entry of the nexthop router > */ > rcu_read_lock(); > - rt = (struct rt6_info *) dst; > + rt = (struct rt6_info *) *dst; > n = rt->n; > if (n && !(n->nud_state & NUD_VALID)) { > struct inet6_ifaddr *ifp; > David, what do you think if I submit a patch using following accessor ? /* get a rt6_info given a dst_entry pointer */ static inline struct rt6_info *dst_rt6_info(struct dst_entry *dst) { return (struct rt6_info *)dst; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 06 Jul 2012 08:13:47 +0200 > @@ -975,7 +975,7 @@ static int ip6_dst_lookup_tail(struct sock *sk, > * dst entry of the nexthop router > */ > rcu_read_lock(); > - rt = (struct rt6_info *) dst; > + rt = (struct rt6_info *) *dst; > n = rt->n; > if (n && !(n->nud_state & NUD_VALID)) { > struct inet6_ifaddr *ifp; This is obviously correct, please submit this formally. Fengguang Wu can I ask you politely not to quote the quilty patch in it's entirety when reporting bugs? That screws up my workflow because that patch goes then gets installed as a new patch in patchwork and I have to therefore tick it off every time you report a bug. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 06 Jul 2012 08:41:47 +0200 > David, what do you think if I submit a patch using following accessor ? > > /* get a rt6_info given a dst_entry pointer */ > static inline struct rt6_info *dst_rt6_info(struct dst_entry *dst) > { > return (struct rt6_info *)dst; > } I'd rather we simply not use address-of pointers in our interfaces like we do now in some spots. 99 times out of 100 it's a case where PTR_ERR() would do. I spent a lot of time moving both ipv4 and ipv6 in this direction, we're almost there, and should simply finish off the remaining cases. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-07-05 at 23:44 -0700, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Fri, 06 Jul 2012 08:41:47 +0200 > > > David, what do you think if I submit a patch using following accessor ? > > > > /* get a rt6_info given a dst_entry pointer */ > > static inline struct rt6_info *dst_rt6_info(struct dst_entry *dst) > > { > > return (struct rt6_info *)dst; > > } > > I'd rather we simply not use address-of pointers in our interfaces > like we do now in some spots. > > 99 times out of 100 it's a case where PTR_ERR() would do. > > I spent a lot of time moving both ipv4 and ipv6 in this direction, > we're almost there, and should simply finish off the remaining > cases. Not sure what you mean. I dont use address of pointer. I suggested a type safe thing ie change all struct rt6_info *rt = (struct rt6_info *)dst; by struct rt6_info *rt = dst_rt6_info(dst); same generated code, but we have compiler checks instead of a raw cast. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 06 Jul 2012 08:53:07 +0200 > Not sure what you mean. I dont use address of pointer. I'm talking about the cases where this caused a bug here. Passing "struct dst_entry **dst" as an argument. It's just stupid, and a relic of code that wants to return a route and a locallized error at the same time. I'm saying that in the end we should simply return "struct dst_entry *" from such functions. Anyways, please submit your original patch in this thread so I can get this simple case fixed in net-next. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, > Fengguang Wu can I ask you politely not to quote the quilty patch in > it's entirety when reporting bugs? That screws up my workflow because > that patch goes then gets installed as a new patch in patchwork and I > have to therefore tick it off every time you report a bug. Sorry for that! Is it fine to _attach_ the referenced patch, or just a raw diff? Or, the commit SHA and subject are all you want to see? Thanks, Fengguang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 06, 2012 at 03:37:45PM +0800, Fengguang Wu wrote: > Hi David, > > > Fengguang Wu can I ask you politely not to quote the quilty patch in > > it's entirety when reporting bugs? That screws up my workflow because > > that patch goes then gets installed as a new patch in patchwork and I > > have to therefore tick it off every time you report a bug. > > Sorry for that! Is it fine to _attach_ the referenced patch, or just > a raw diff? Or, the commit SHA and subject are all you want to see? I used git-format-patch which makes a formal patch. How about git-show? The output will be less like a formal patch, for example: : commit c5fb75aafab2fe31353b96cf556c1a689f8ac7e9 : Author: Fengguang Wu <fengguang.wu@intel.com> : Date: Thu Jun 14 22:36:29 2012 +0800 : pms: fix build error in pms_probe() : : drivers/media/video/pms.c: In function ‘pms_probe’: : drivers/media/video/pms.c:1047:2: error: implicit declaration of function ‘kzalloc’ [-Werror=implicit-function-declaration] : drivers/media/video/pms.c:1047:6: warning: assignment makes pointer from integer without a cast [enabled by default] : drivers/media/video/pms.c:1116:2: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration] : : Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> : diff --git a/drivers/media/video/pms.c b/drivers/media/video/pms.c : index af2d908..77f9c92 100644 : --- a/drivers/media/video/pms.c : +++ b/drivers/media/video/pms.c : @@ -26,6 +26,7 @@ : #include <linux/fs.h> : #include <linux/kernel.h> : #include <linux/mm.h> : +#include <linux/slab.h> : #include <linux/ioport.h> : #include <linux/init.h> : #include <linux/mutex.h> Thanks, Fengguang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Fengguang Wu <wfg@linux.intel.com> Date: Fri, 6 Jul 2012 15:37:45 +0800 > Sorry for that! Is it fine to _attach_ the referenced patch, or just > a raw diff? Or, the commit SHA and subject are all you want to see? Don't provide the diff at all, in any form. Even if you attach it patchwork can still parse it and queued it up. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Fengguang Wu <wfg@linux.intel.com> Date: Fri, 6 Jul 2012 15:52:45 +0800 > On Fri, Jul 06, 2012 at 03:37:45PM +0800, Fengguang Wu wrote: >> Hi David, >> >> > Fengguang Wu can I ask you politely not to quote the quilty patch in >> > it's entirety when reporting bugs? That screws up my workflow because >> > that patch goes then gets installed as a new patch in patchwork and I >> > have to therefore tick it off every time you report a bug. >> >> Sorry for that! Is it fine to _attach_ the referenced patch, or just >> a raw diff? Or, the commit SHA and subject are all you want to see? > > I used git-format-patch which makes a formal patch. How about git-show? > The output will be less like a formal patch, for example: No patch, in any format. It's completely pointless to attach the diff, anyone can use the commit log message and SHA ID to fetch the patch if they want. It's redundancy therefore also makes it a huge waste of bandwidth. I have no idea why you provide it in the first place. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 06, 2012 at 01:29:21AM -0700, David Miller wrote: > From: Fengguang Wu <wfg@linux.intel.com> > Date: Fri, 6 Jul 2012 15:52:45 +0800 > > > On Fri, Jul 06, 2012 at 03:37:45PM +0800, Fengguang Wu wrote: > >> Hi David, > >> > >> > Fengguang Wu can I ask you politely not to quote the quilty patch in > >> > it's entirety when reporting bugs? That screws up my workflow because > >> > that patch goes then gets installed as a new patch in patchwork and I > >> > have to therefore tick it off every time you report a bug. > >> > >> Sorry for that! Is it fine to _attach_ the referenced patch, or just > >> a raw diff? Or, the commit SHA and subject are all you want to see? > > > > I used git-format-patch which makes a formal patch. How about git-show? > > The output will be less like a formal patch, for example: > > No patch, in any format. > > It's completely pointless to attach the diff, anyone can use the > commit log message and SHA ID to fetch the patch if they want. OK! > It's redundancy therefore also makes it a huge waste of bandwidth. I > have no idea why you provide it in the first place. I find it very convenient on myself for confirming the error/warning.. Thanks, Fengguang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 6d9c0ab..c6af596 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -975,7 +975,7 @@ static int ip6_dst_lookup_tail(struct sock *sk, * dst entry of the nexthop router */ rcu_read_lock(); - rt = (struct rt6_info *) dst; + rt = (struct rt6_info *) *dst; n = rt->n; if (n && !(n->nud_state & NUD_VALID)) { struct inet6_ifaddr *ifp;