[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <52A2A9A1.7000402@huawei.com>
Date: Sat, 7 Dec 2013 12:52:49 +0800
From: Wang Weidong <wangweidong1@...wei.com>
To: Jon Maloy <jon.maloy@...csson.com>
CC: Ying Xue <ying.xue@...driver.com>, <allan.stephens@...driver.com>,
<davem@...emloft.net>, <netdev@...r.kernel.org>,
<tipc-discussion@...ts.sourceforge.net>
Subject: Re: [PATCH net-next 1/6] tipc: add link_kfree_skbuff helper function
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@...wei.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists