| Message ID | 1386311022-11176-2-git-send-email-wangweidong1@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 12/06/2013 02:23 PM, Wang Weidong wrote: > replaces some chunks of code that kfree the sk_buff. > This is just code simplification, no functional changes. > > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- > 1 file changed, 19 insertions(+), 39 deletions(-) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index 69cd9bf..1c27d7b 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) > return (i + 3) & ~3u; > } > > +static void link_kfree_skbuff(struct sk_buff *buf) > +{ > + struct sk_buff *next; > + > + while (buf) { > + next = buf->next; > + kfree_skb(buf); > + buf = next; > + } > +} > + Your new defined function is unnecessary, instead we already have another patch doing the same thing with kfree_skb_list(), and the patch will be to be sent out soon. Please see below link: http://article.gmane.org/gmane.network.tipc.general/5140/ And the patch cleans up more things than your patch. Regards, Ying > static void link_init_max_pkt(struct tipc_link *l_ptr) > { > u32 max_pkt; > @@ -387,13 +398,8 @@ exit: > static void link_release_outqueue(struct tipc_link *l_ptr) > { > struct sk_buff *buf = l_ptr->first_out; > - struct sk_buff *next; > > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > l_ptr->first_out = NULL; > l_ptr->out_queue_size = 0; > } > @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) > void tipc_link_stop(struct tipc_link *l_ptr) > { > struct sk_buff *buf; > - struct sk_buff *next; > > buf = l_ptr->oldest_deferred_in; > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > > buf = l_ptr->first_out; > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > > tipc_link_reset_fragments(l_ptr); > > @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) > kfree_skb(l_ptr->proto_msg_queue); > l_ptr->proto_msg_queue = NULL; > buf = l_ptr->oldest_deferred_in; > - while (buf) { > - struct sk_buff *next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > if (!list_empty(&l_ptr->waiting_ports)) > tipc_link_wakeup_ports(l_ptr, 1); > > @@ -1127,10 +1120,7 @@ again: > if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { > res = -EFAULT; > error: > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > return res; > } > sect_crs += sz; > @@ -1180,18 +1170,12 @@ error: > if (l_ptr->max_pkt < max_pkt) { > sender->max_pkt = l_ptr->max_pkt; > tipc_node_unlock(node); > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > goto again; > } > } else { > reject: > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > return tipc_port_reject_sections(sender, hdr, msg_sect, > len, TIPC_ERR_NO_NODE); > } > @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) > fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); > if (fragm == NULL) { > kfree_skb(buf); > - while (buf_chain) { > - buf = buf_chain; > - buf_chain = buf_chain->next; > - kfree_skb(buf); > - } > + link_kfree_skbuff(buf_chain); > return -ENOMEM; > } > msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); > -- 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 2013/12/6 14:34, Ying Xue wrote: > On 12/06/2013 02:23 PM, Wang Weidong wrote: >> replaces some chunks of code that kfree the sk_buff. >> This is just code simplification, no functional changes. >> >> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >> --- >> net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- >> 1 file changed, 19 insertions(+), 39 deletions(-) >> >> diff --git a/net/tipc/link.c b/net/tipc/link.c >> index 69cd9bf..1c27d7b 100644 >> --- a/net/tipc/link.c >> +++ b/net/tipc/link.c >> @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) >> return (i + 3) & ~3u; >> } >> >> +static void link_kfree_skbuff(struct sk_buff *buf) >> +{ >> + struct sk_buff *next; >> + >> + while (buf) { >> + next = buf->next; >> + kfree_skb(buf); >> + buf = next; >> + } >> +} >> + > > Your new defined function is unnecessary, instead we already have > another patch doing the same thing with kfree_skb_list(), and the patch > will be to be sent out soon. > > Please see below link: > > http://article.gmane.org/gmane.network.tipc.general/5140/ > > And the patch cleans up more things than your patch. > Yes, You are right. Thanks. > Regards, > Ying > >> static void link_init_max_pkt(struct tipc_link *l_ptr) >> { >> u32 max_pkt; >> @@ -387,13 +398,8 @@ exit: >> static void link_release_outqueue(struct tipc_link *l_ptr) >> { >> struct sk_buff *buf = l_ptr->first_out; >> - struct sk_buff *next; >> >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> l_ptr->first_out = NULL; >> l_ptr->out_queue_size = 0; >> } >> @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) >> void tipc_link_stop(struct tipc_link *l_ptr) >> { >> struct sk_buff *buf; >> - struct sk_buff *next; >> >> buf = l_ptr->oldest_deferred_in; >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> >> buf = l_ptr->first_out; >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> >> tipc_link_reset_fragments(l_ptr); >> >> @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) >> kfree_skb(l_ptr->proto_msg_queue); >> l_ptr->proto_msg_queue = NULL; >> buf = l_ptr->oldest_deferred_in; >> - while (buf) { >> - struct sk_buff *next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> if (!list_empty(&l_ptr->waiting_ports)) >> tipc_link_wakeup_ports(l_ptr, 1); >> >> @@ -1127,10 +1120,7 @@ again: >> if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { >> res = -EFAULT; >> error: >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> return res; >> } >> sect_crs += sz; >> @@ -1180,18 +1170,12 @@ error: >> if (l_ptr->max_pkt < max_pkt) { >> sender->max_pkt = l_ptr->max_pkt; >> tipc_node_unlock(node); >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> goto again; >> } >> } else { >> reject: >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> return tipc_port_reject_sections(sender, hdr, msg_sect, >> len, TIPC_ERR_NO_NODE); >> } >> @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) >> fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); >> if (fragm == NULL) { >> kfree_skb(buf); >> - while (buf_chain) { >> - buf = buf_chain; >> - buf_chain = buf_chain->next; >> - kfree_skb(buf); >> - } >> + link_kfree_skbuff(buf_chain); >> return -ENOMEM; >> } >> msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); >> > > > . > -- 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, 2013-12-06 at 14:23 +0800, Wang Weidong wrote: > replaces some chunks of code that kfree the sk_buff. > This is just code simplification, no functional changes. > > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- > 1 file changed, 19 insertions(+), 39 deletions(-) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index 69cd9bf..1c27d7b 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) > return (i + 3) & ~3u; > } > > +static void link_kfree_skbuff(struct sk_buff *buf) > +{ > + struct sk_buff *next; > + > + while (buf) { > + next = buf->next; > + kfree_skb(buf); > + buf = next; > + } > +} > + Oh well, this looks like kfree_skb_list() to me. -- 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 12/06/2013 07:23 AM, Wang Weidong wrote: > replaces some chunks of code that kfree the sk_buff. > This is just code simplification, no functional changes. > > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- > 1 file changed, 19 insertions(+), 39 deletions(-) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index 69cd9bf..1c27d7b 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) > return (i + 3) & ~3u; > } > > +static void link_kfree_skbuff(struct sk_buff *buf) > +{ > + struct sk_buff *next; > + > + while (buf) { > + next = buf->next; > + kfree_skb(buf); > + buf = next; > + } > +} So kfree_skb_list() does not work for you ? Also, in case we do not have such generic functions, they should go to skbuff.c instead ... > static void link_init_max_pkt(struct tipc_link *l_ptr) > { > u32 max_pkt; > @@ -387,13 +398,8 @@ exit: > static void link_release_outqueue(struct tipc_link *l_ptr) > { > struct sk_buff *buf = l_ptr->first_out; > - struct sk_buff *next; > > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > l_ptr->first_out = NULL; > l_ptr->out_queue_size = 0; > } > @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) > void tipc_link_stop(struct tipc_link *l_ptr) > { > struct sk_buff *buf; > - struct sk_buff *next; > > buf = l_ptr->oldest_deferred_in; > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > > buf = l_ptr->first_out; > - while (buf) { > - next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > > tipc_link_reset_fragments(l_ptr); > > @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) > kfree_skb(l_ptr->proto_msg_queue); > l_ptr->proto_msg_queue = NULL; > buf = l_ptr->oldest_deferred_in; > - while (buf) { > - struct sk_buff *next = buf->next; > - kfree_skb(buf); > - buf = next; > - } > + link_kfree_skbuff(buf); > if (!list_empty(&l_ptr->waiting_ports)) > tipc_link_wakeup_ports(l_ptr, 1); > > @@ -1127,10 +1120,7 @@ again: > if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { > res = -EFAULT; > error: > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > return res; > } > sect_crs += sz; > @@ -1180,18 +1170,12 @@ error: > if (l_ptr->max_pkt < max_pkt) { > sender->max_pkt = l_ptr->max_pkt; > tipc_node_unlock(node); > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > goto again; > } > } else { > reject: > - for (; buf_chain; buf_chain = buf) { > - buf = buf_chain->next; > - kfree_skb(buf_chain); > - } > + link_kfree_skbuff(buf_chain); > return tipc_port_reject_sections(sender, hdr, msg_sect, > len, TIPC_ERR_NO_NODE); > } > @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) > fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); > if (fragm == NULL) { > kfree_skb(buf); > - while (buf_chain) { > - buf = buf_chain; > - buf_chain = buf_chain->next; > - kfree_skb(buf); > - } > + link_kfree_skbuff(buf_chain); > return -ENOMEM; > } > msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); > -- 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 2013/12/6 17:09, Daniel Borkmann wrote: > On 12/06/2013 07:23 AM, Wang Weidong wrote: >> replaces some chunks of code that kfree the sk_buff. >> This is just code simplification, no functional changes. >> >> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >> --- >> net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- >> 1 file changed, 19 insertions(+), 39 deletions(-) >> >> diff --git a/net/tipc/link.c b/net/tipc/link.c >> index 69cd9bf..1c27d7b 100644 >> --- a/net/tipc/link.c >> +++ b/net/tipc/link.c >> @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) >> return (i + 3) & ~3u; >> } >> >> +static void link_kfree_skbuff(struct sk_buff *buf) >> +{ >> + struct sk_buff *next; >> + >> + while (buf) { >> + next = buf->next; >> + kfree_skb(buf); >> + buf = next; >> + } >> +} > > So kfree_skb_list() does not work for you ? > > Also, in case we do not have such generic functions, they should > go to skbuff.c instead ... > No, It is work for me. I just din't not the kfree_skb_list() and I found that use a helper function can make code more simplification. So I do that. Regards. Wang >> static void link_init_max_pkt(struct tipc_link *l_ptr) >> { >> u32 max_pkt; >> @@ -387,13 +398,8 @@ exit: >> static void link_release_outqueue(struct tipc_link *l_ptr) >> { >> struct sk_buff *buf = l_ptr->first_out; >> - struct sk_buff *next; >> >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> l_ptr->first_out = NULL; >> l_ptr->out_queue_size = 0; >> } >> @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) >> void tipc_link_stop(struct tipc_link *l_ptr) >> { >> struct sk_buff *buf; >> - struct sk_buff *next; >> >> buf = l_ptr->oldest_deferred_in; >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> >> buf = l_ptr->first_out; >> - while (buf) { >> - next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> >> tipc_link_reset_fragments(l_ptr); >> >> @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) >> kfree_skb(l_ptr->proto_msg_queue); >> l_ptr->proto_msg_queue = NULL; >> buf = l_ptr->oldest_deferred_in; >> - while (buf) { >> - struct sk_buff *next = buf->next; >> - kfree_skb(buf); >> - buf = next; >> - } >> + link_kfree_skbuff(buf); >> if (!list_empty(&l_ptr->waiting_ports)) >> tipc_link_wakeup_ports(l_ptr, 1); >> >> @@ -1127,10 +1120,7 @@ again: >> if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { >> res = -EFAULT; >> error: >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> return res; >> } >> sect_crs += sz; >> @@ -1180,18 +1170,12 @@ error: >> if (l_ptr->max_pkt < max_pkt) { >> sender->max_pkt = l_ptr->max_pkt; >> tipc_node_unlock(node); >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> goto again; >> } >> } else { >> reject: >> - for (; buf_chain; buf_chain = buf) { >> - buf = buf_chain->next; >> - kfree_skb(buf_chain); >> - } >> + link_kfree_skbuff(buf_chain); >> return tipc_port_reject_sections(sender, hdr, msg_sect, >> len, TIPC_ERR_NO_NODE); >> } >> @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) >> fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); >> if (fragm == NULL) { >> kfree_skb(buf); >> - while (buf_chain) { >> - buf = buf_chain; >> - buf_chain = buf_chain->next; >> - kfree_skb(buf); >> - } >> + link_kfree_skbuff(buf_chain); >> return -ENOMEM; >> } >> msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); >> > > . > -- 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
Wang, I am very happy to see you posting improvements to TIPC, but please synch up with the TIPC development team (i.e., use tipc_discussion), before posting it to netdev. As Ying stated,we have a patch series in the pipe that deals with exactly this issue, and more. Regards ///jon On 12/06/2013 01:42 AM, Wang Weidong wrote: > On 2013/12/6 14:34, Ying Xue wrote: >> On 12/06/2013 02:23 PM, Wang Weidong wrote: >>> replaces some chunks of code that kfree the sk_buff. >>> This is just code simplification, no functional changes. >>> >>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >>> --- >>> net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- >>> 1 file changed, 19 insertions(+), 39 deletions(-) >>> >>> diff --git a/net/tipc/link.c b/net/tipc/link.c >>> index 69cd9bf..1c27d7b 100644 >>> --- a/net/tipc/link.c >>> +++ b/net/tipc/link.c >>> @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) >>> return (i + 3) & ~3u; >>> } >>> >>> +static void link_kfree_skbuff(struct sk_buff *buf) >>> +{ >>> + struct sk_buff *next; >>> + >>> + while (buf) { >>> + next = buf->next; >>> + kfree_skb(buf); >>> + buf = next; >>> + } >>> +} >>> + >> >> Your new defined function is unnecessary, instead we already have >> another patch doing the same thing with kfree_skb_list(), and the patch >> will be to be sent out soon. >> >> Please see below link: >> >> http://article.gmane.org/gmane.network.tipc.general/5140/ >> >> And the patch cleans up more things than your patch. >> > > Yes, You are right. > Thanks. > >> Regards, >> Ying >> >>> static void link_init_max_pkt(struct tipc_link *l_ptr) >>> { >>> u32 max_pkt; >>> @@ -387,13 +398,8 @@ exit: >>> static void link_release_outqueue(struct tipc_link *l_ptr) >>> { >>> struct sk_buff *buf = l_ptr->first_out; >>> - struct sk_buff *next; >>> >>> - while (buf) { >>> - next = buf->next; >>> - kfree_skb(buf); >>> - buf = next; >>> - } >>> + link_kfree_skbuff(buf); >>> l_ptr->first_out = NULL; >>> l_ptr->out_queue_size = 0; >>> } >>> @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) >>> void tipc_link_stop(struct tipc_link *l_ptr) >>> { >>> struct sk_buff *buf; >>> - struct sk_buff *next; >>> >>> buf = l_ptr->oldest_deferred_in; >>> - while (buf) { >>> - next = buf->next; >>> - kfree_skb(buf); >>> - buf = next; >>> - } >>> + link_kfree_skbuff(buf); >>> >>> buf = l_ptr->first_out; >>> - while (buf) { >>> - next = buf->next; >>> - kfree_skb(buf); >>> - buf = next; >>> - } >>> + link_kfree_skbuff(buf); >>> >>> tipc_link_reset_fragments(l_ptr); >>> >>> @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) >>> kfree_skb(l_ptr->proto_msg_queue); >>> l_ptr->proto_msg_queue = NULL; >>> buf = l_ptr->oldest_deferred_in; >>> - while (buf) { >>> - struct sk_buff *next = buf->next; >>> - kfree_skb(buf); >>> - buf = next; >>> - } >>> + link_kfree_skbuff(buf); >>> if (!list_empty(&l_ptr->waiting_ports)) >>> tipc_link_wakeup_ports(l_ptr, 1); >>> >>> @@ -1127,10 +1120,7 @@ again: >>> if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { >>> res = -EFAULT; >>> error: >>> - for (; buf_chain; buf_chain = buf) { >>> - buf = buf_chain->next; >>> - kfree_skb(buf_chain); >>> - } >>> + link_kfree_skbuff(buf_chain); >>> return res; >>> } >>> sect_crs += sz; >>> @@ -1180,18 +1170,12 @@ error: >>> if (l_ptr->max_pkt < max_pkt) { >>> sender->max_pkt = l_ptr->max_pkt; >>> tipc_node_unlock(node); >>> - for (; buf_chain; buf_chain = buf) { >>> - buf = buf_chain->next; >>> - kfree_skb(buf_chain); >>> - } >>> + link_kfree_skbuff(buf_chain); >>> goto again; >>> } >>> } else { >>> reject: >>> - for (; buf_chain; buf_chain = buf) { >>> - buf = buf_chain->next; >>> - kfree_skb(buf_chain); >>> - } >>> + link_kfree_skbuff(buf_chain); >>> return tipc_port_reject_sections(sender, hdr, msg_sect, >>> len, TIPC_ERR_NO_NODE); >>> } >>> @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) >>> fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); >>> if (fragm == NULL) { >>> kfree_skb(buf); >>> - while (buf_chain) { >>> - buf = buf_chain; >>> - buf_chain = buf_chain->next; >>> - kfree_skb(buf); >>> - } >>> + link_kfree_skbuff(buf_chain); >>> return -ENOMEM; >>> } >>> msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); >>> >> >> >> . >> > > -- 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 2013/12/6 22:13, Jon Maloy wrote: > Wang, > I am very happy to see you posting improvements to TIPC, but please synch up > with the TIPC development team (i.e., use tipc_discussion), before posting > it to netdev. As Ying stated,we have a patch series in the pipe that deals > with exactly this issue, and more. > > Regards > ///jon > Hi Jon, Thanks for your reply. Now I am more clearly about how to do it. Regards. Wang > > > > On 12/06/2013 01:42 AM, Wang Weidong wrote: >> On 2013/12/6 14:34, Ying Xue wrote: >>> On 12/06/2013 02:23 PM, Wang Weidong wrote: >>>> replaces some chunks of code that kfree the sk_buff. >>>> This is just code simplification, no functional changes. >>>> >>>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >>>> --- >>>> net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- >>>> 1 file changed, 19 insertions(+), 39 deletions(-) >>>> >>>> diff --git a/net/tipc/link.c b/net/tipc/link.c >>>> index 69cd9bf..1c27d7b 100644 >>>> --- a/net/tipc/link.c >>>> +++ b/net/tipc/link.c >>>> @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) >>>> return (i + 3) & ~3u; >>>> } >>>> >>>> +static void link_kfree_skbuff(struct sk_buff *buf) >>>> +{ >>>> + struct sk_buff *next; >>>> + >>>> + while (buf) { >>>> + next = buf->next; >>>> + kfree_skb(buf); >>>> + buf = next; >>>> + } >>>> +} >>>> + >>> >>> Your new defined function is unnecessary, instead we already have >>> another patch doing the same thing with kfree_skb_list(), and the patch >>> will be to be sent out soon. >>> >>> Please see below link: >>> >>> http://article.gmane.org/gmane.network.tipc.general/5140/ >>> >>> And the patch cleans up more things than your patch. >>> >> >> Yes, You are right. >> Thanks. >> >>> Regards, >>> Ying >>> >>>> static void link_init_max_pkt(struct tipc_link *l_ptr) >>>> { >>>> u32 max_pkt; >>>> @@ -387,13 +398,8 @@ exit: >>>> static void link_release_outqueue(struct tipc_link *l_ptr) >>>> { >>>> struct sk_buff *buf = l_ptr->first_out; >>>> - struct sk_buff *next; >>>> >>>> - while (buf) { >>>> - next = buf->next; >>>> - kfree_skb(buf); >>>> - buf = next; >>>> - } >>>> + link_kfree_skbuff(buf); >>>> l_ptr->first_out = NULL; >>>> l_ptr->out_queue_size = 0; >>>> } >>>> @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) >>>> void tipc_link_stop(struct tipc_link *l_ptr) >>>> { >>>> struct sk_buff *buf; >>>> - struct sk_buff *next; >>>> >>>> buf = l_ptr->oldest_deferred_in; >>>> - while (buf) { >>>> - next = buf->next; >>>> - kfree_skb(buf); >>>> - buf = next; >>>> - } >>>> + link_kfree_skbuff(buf); >>>> >>>> buf = l_ptr->first_out; >>>> - while (buf) { >>>> - next = buf->next; >>>> - kfree_skb(buf); >>>> - buf = next; >>>> - } >>>> + link_kfree_skbuff(buf); >>>> >>>> tipc_link_reset_fragments(l_ptr); >>>> >>>> @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) >>>> kfree_skb(l_ptr->proto_msg_queue); >>>> l_ptr->proto_msg_queue = NULL; >>>> buf = l_ptr->oldest_deferred_in; >>>> - while (buf) { >>>> - struct sk_buff *next = buf->next; >>>> - kfree_skb(buf); >>>> - buf = next; >>>> - } >>>> + link_kfree_skbuff(buf); >>>> if (!list_empty(&l_ptr->waiting_ports)) >>>> tipc_link_wakeup_ports(l_ptr, 1); >>>> >>>> @@ -1127,10 +1120,7 @@ again: >>>> if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { >>>> res = -EFAULT; >>>> error: >>>> - for (; buf_chain; buf_chain = buf) { >>>> - buf = buf_chain->next; >>>> - kfree_skb(buf_chain); >>>> - } >>>> + link_kfree_skbuff(buf_chain); >>>> return res; >>>> } >>>> sect_crs += sz; >>>> @@ -1180,18 +1170,12 @@ error: >>>> if (l_ptr->max_pkt < max_pkt) { >>>> sender->max_pkt = l_ptr->max_pkt; >>>> tipc_node_unlock(node); >>>> - for (; buf_chain; buf_chain = buf) { >>>> - buf = buf_chain->next; >>>> - kfree_skb(buf_chain); >>>> - } >>>> + link_kfree_skbuff(buf_chain); >>>> goto again; >>>> } >>>> } else { >>>> reject: >>>> - for (; buf_chain; buf_chain = buf) { >>>> - buf = buf_chain->next; >>>> - kfree_skb(buf_chain); >>>> - } >>>> + link_kfree_skbuff(buf_chain); >>>> return tipc_port_reject_sections(sender, hdr, msg_sect, >>>> len, TIPC_ERR_NO_NODE); >>>> } >>>> @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) >>>> fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); >>>> if (fragm == NULL) { >>>> kfree_skb(buf); >>>> - while (buf_chain) { >>>> - buf = buf_chain; >>>> - buf_chain = buf_chain->next; >>>> - kfree_skb(buf); >>>> - } >>>> + link_kfree_skbuff(buf_chain); >>>> return -ENOMEM; >>>> } >>>> msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE); >>>> >>> >>> >>> . >>> >> >> > > > . > -- 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/tipc/link.c b/net/tipc/link.c index 69cd9bf..1c27d7b 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -100,6 +100,17 @@ static unsigned int align(unsigned int i) return (i + 3) & ~3u; } +static void link_kfree_skbuff(struct sk_buff *buf) +{ + struct sk_buff *next; + + while (buf) { + next = buf->next; + kfree_skb(buf); + buf = next; + } +} + static void link_init_max_pkt(struct tipc_link *l_ptr) { u32 max_pkt; @@ -387,13 +398,8 @@ exit: static void link_release_outqueue(struct tipc_link *l_ptr) { struct sk_buff *buf = l_ptr->first_out; - struct sk_buff *next; - while (buf) { - next = buf->next; - kfree_skb(buf); - buf = next; - } + link_kfree_skbuff(buf); l_ptr->first_out = NULL; l_ptr->out_queue_size = 0; } @@ -416,21 +422,12 @@ void tipc_link_reset_fragments(struct tipc_link *l_ptr) void tipc_link_stop(struct tipc_link *l_ptr) { struct sk_buff *buf; - struct sk_buff *next; buf = l_ptr->oldest_deferred_in; - while (buf) { - next = buf->next; - kfree_skb(buf); - buf = next; - } + link_kfree_skbuff(buf); buf = l_ptr->first_out; - while (buf) { - next = buf->next; - kfree_skb(buf); - buf = next; - } + link_kfree_skbuff(buf); tipc_link_reset_fragments(l_ptr); @@ -472,11 +469,7 @@ void tipc_link_reset(struct tipc_link *l_ptr) kfree_skb(l_ptr->proto_msg_queue); l_ptr->proto_msg_queue = NULL; buf = l_ptr->oldest_deferred_in; - while (buf) { - struct sk_buff *next = buf->next; - kfree_skb(buf); - buf = next; - } + link_kfree_skbuff(buf); if (!list_empty(&l_ptr->waiting_ports)) tipc_link_wakeup_ports(l_ptr, 1); @@ -1127,10 +1120,7 @@ again: if (copy_from_user(buf->data + fragm_crs, sect_crs, sz)) { res = -EFAULT; error: - for (; buf_chain; buf_chain = buf) { - buf = buf_chain->next; - kfree_skb(buf_chain); - } + link_kfree_skbuff(buf_chain); return res; } sect_crs += sz; @@ -1180,18 +1170,12 @@ error: if (l_ptr->max_pkt < max_pkt) { sender->max_pkt = l_ptr->max_pkt; tipc_node_unlock(node); - for (; buf_chain; buf_chain = buf) { - buf = buf_chain->next; - kfree_skb(buf_chain); - } + link_kfree_skbuff(buf_chain); goto again; } } else { reject: - for (; buf_chain; buf_chain = buf) { - buf = buf_chain->next; - kfree_skb(buf_chain); - } + link_kfree_skbuff(buf_chain); return tipc_port_reject_sections(sender, hdr, msg_sect, len, TIPC_ERR_NO_NODE); } @@ -2306,11 +2290,7 @@ static int link_send_long_buf(struct tipc_link *l_ptr, struct sk_buff *buf) fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE); if (fragm == NULL) { kfree_skb(buf); - while (buf_chain) { - buf = buf_chain; - buf_chain = buf_chain->next; - kfree_skb(buf); - } + link_kfree_skbuff(buf_chain); return -ENOMEM; } msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE);
replaces some chunks of code that kfree the sk_buff. This is just code simplification, no functional changes. Signed-off-by: Wang Weidong <wangweidong1@huawei.com> --- net/tipc/link.c | 58 +++++++++++++++++++-------------------------------------- 1 file changed, 19 insertions(+), 39 deletions(-)