[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1416598774.24312.9@mail.thefacebook.com>
Date: Fri, 21 Nov 2014 14:39:34 -0500
From: Chris Mason <clm@...com>
To: Eric Dumazet <eric.dumazet@...il.com>
CC: Sabrina Dubroca <sd@...asysnail.net>,
David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: Revert "net: avoid one atomic operation in
skb_clone()"
On Fri, Nov 21, 2014 at 2:29 PM, Eric Dumazet <eric.dumazet@...il.com>
wrote:
> On Fri, 2014-11-21 at 19:05 +0100, Sabrina Dubroca wrote:
>> Hello Eric,
>>
>> >
>> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> > index c16615bfb61e..be4c7deed971 100644
>> > --- a/net/core/skbuff.c
>> > +++ b/net/core/skbuff.c
>> > @@ -552,20 +552,13 @@ static void kfree_skbmem(struct sk_buff
>> *skb)
>> > case SKB_FCLONE_CLONE:
>> > fclones = container_of(skb, struct sk_buff_fclones, skb2);
>> >
>> > - /* Warning : We must perform the atomic_dec_and_test() before
>> > - * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
>> > - * skb_clone() could set clone_ref to 2 before our decrement.
>> > - * Anyway, if we are going to free the structure, no need to
>> > - * rewrite skb->fclone.
>> > + /* The clone portion is available for
>> > + * fast-cloning again.
>> > */
>> > - if (atomic_dec_and_test(&fclones->fclone_ref)) {
>> > + skb->fclone = SKB_FCLONE_UNAVAILABLE;
>>
>> Shouldn't that be SKB_FCLONE_FREE ?
>
> Absolutely :(
>
> I'll send a v2, thanks !
v1 made it through the first pass here. I'll switch to SKB_FCLONE_FREE
and restart.
-chris
--
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