* [PATCH net-next] myri10ge: fix truesize underestimation
@ 2011-10-20 20:10 Eric Dumazet
2011-10-20 20:33 ` Jon Mason
2011-10-20 21:42 ` David Miller
0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2011-10-20 20:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Jon Mason
skb->truesize must account for allocated memory, not the used part of
it. Doing this work is important to avoid unexpected OOM situations.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jon Mason <mason@myri.com>
---
drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index c970a48..0778edc 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1210,7 +1210,6 @@ myri10ge_rx_skb_build(struct sk_buff *skb, u8 * va,
struct skb_frag_struct *skb_frags;
skb->len = skb->data_len = len;
- skb->truesize = len + sizeof(struct sk_buff);
/* attach the page(s) */
skb_frags = skb_shinfo(skb)->frags;
@@ -1385,6 +1384,8 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
if (skb_frag_size(&skb_shinfo(skb)->frags[0]) <= 0) {
skb_frag_unref(skb, 0);
skb_shinfo(skb)->nr_frags = 0;
+ } else {
+ skb->truesize += bytes * skb_shinfo(skb)->nr_frags;
}
skb->protocol = eth_type_trans(skb, dev);
skb_record_rx_queue(skb, ss - &mgp->ss[0]);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:10 [PATCH net-next] myri10ge: fix truesize underestimation Eric Dumazet
@ 2011-10-20 20:33 ` Jon Mason
2011-10-20 20:44 ` Eric Dumazet
2011-10-20 21:42 ` David Miller
1 sibling, 1 reply; 9+ messages in thread
From: Jon Mason @ 2011-10-20 20:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Andrew Gallatin
On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> skb->truesize must account for allocated memory, not the used part of
> it. Doing this work is important to avoid unexpected OOM situations.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Jon Mason <mason@myri.com>
> CC: Jon Mason <mason@myri.com>
> ---
> drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> index c970a48..0778edc 100644
> --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> @@ -1210,7 +1210,6 @@ myri10ge_rx_skb_build(struct sk_buff *skb, u8 * va,
> struct skb_frag_struct *skb_frags;
>
> skb->len = skb->data_len = len;
> - skb->truesize = len + sizeof(struct sk_buff);
> /* attach the page(s) */
>
> skb_frags = skb_shinfo(skb)->frags;
> @@ -1385,6 +1384,8 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
> if (skb_frag_size(&skb_shinfo(skb)->frags[0]) <= 0) {
> skb_frag_unref(skb, 0);
> skb_shinfo(skb)->nr_frags = 0;
> + } else {
> + skb->truesize += bytes * skb_shinfo(skb)->nr_frags;
> }
> skb->protocol = eth_type_trans(skb, dev);
> skb_record_rx_queue(skb, ss - &mgp->ss[0]);
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:33 ` Jon Mason
@ 2011-10-20 20:44 ` Eric Dumazet
2011-10-20 20:45 ` Andrew Gallatin
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2011-10-20 20:44 UTC (permalink / raw)
To: Jon Mason; +Cc: David Miller, netdev, Andrew Gallatin
Le jeudi 20 octobre 2011 à 15:33 -0500, Jon Mason a écrit :
> On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > skb->truesize must account for allocated memory, not the used part of
> > it. Doing this work is important to avoid unexpected OOM situations.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Acked-by: Jon Mason <mason@myri.com>
Thanks for reviewing Jon !
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:44 ` Eric Dumazet
@ 2011-10-20 20:45 ` Andrew Gallatin
2011-10-20 20:59 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Gallatin @ 2011-10-20 20:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jon Mason, David Miller, netdev
On 10/20/11 16:44, Eric Dumazet wrote:
> Le jeudi 20 octobre 2011 à 15:33 -0500, Jon Mason a écrit :
>> On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet<eric.dumazet@gmail.com> wrote:
>>> skb->truesize must account for allocated memory, not the used part of
>>> it. Doing this work is important to avoid unexpected OOM situations.
>>>
>>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>>
>> Acked-by: Jon Mason<mason@myri.com>
>
> Thanks for reviewing Jon !
>
>
Please wait a second.. I think the patch is incorrect.
There is already code in myri10ge_rx_skb_build() which
attempts to set the truesize. However, it sets it to
the used, rather than the allocated size so it is apparently
incorrect.
I'd prefer we fix that code.
Thanks,
Drew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:45 ` Andrew Gallatin
@ 2011-10-20 20:59 ` Eric Dumazet
2011-10-20 21:03 ` David Miller
2011-10-20 21:04 ` Andrew Gallatin
0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2011-10-20 20:59 UTC (permalink / raw)
To: Andrew Gallatin; +Cc: Jon Mason, David Miller, netdev
Le jeudi 20 octobre 2011 à 16:45 -0400, Andrew Gallatin a écrit :
> On 10/20/11 16:44, Eric Dumazet wrote:
> > Le jeudi 20 octobre 2011 à 15:33 -0500, Jon Mason a écrit :
> >> On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet<eric.dumazet@gmail.com> wrote:
> >>> skb->truesize must account for allocated memory, not the used part of
> >>> it. Doing this work is important to avoid unexpected OOM situations.
> >>>
> >>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> >>
> >> Acked-by: Jon Mason<mason@myri.com>
> >
> > Thanks for reviewing Jon !
> >
> >
>
> Please wait a second.. I think the patch is incorrect.
>
> There is already code in myri10ge_rx_skb_build() which
> attempts to set the truesize. However, it sets it to
> the used, rather than the allocated size so it is apparently
> incorrect.
>
> I'd prefer we fix that code.
Well, I believe I did exactly that :)
truesize of initial skb is fine.
Then for everay frag added, you must add to skb-truesize the allocated
memory for this frag.
You add frags of a given size (small or big)
In the end, its truesize += bytes * number_of_frags
(bytes being small_size or big_size)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:59 ` Eric Dumazet
@ 2011-10-20 21:03 ` David Miller
2011-10-20 21:04 ` Andrew Gallatin
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2011-10-20 21:03 UTC (permalink / raw)
To: eric.dumazet; +Cc: gallatin, mason, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 20 Oct 2011 22:59:16 +0200
> Le jeudi 20 octobre 2011 à 16:45 -0400, Andrew Gallatin a écrit :
>> On 10/20/11 16:44, Eric Dumazet wrote:
>> > Le jeudi 20 octobre 2011 à 15:33 -0500, Jon Mason a écrit :
>> >> On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet<eric.dumazet@gmail.com> wrote:
>> >>> skb->truesize must account for allocated memory, not the used part of
>> >>> it. Doing this work is important to avoid unexpected OOM situations.
>> >>>
>> >>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>> >>
>> >> Acked-by: Jon Mason<mason@myri.com>
>> >
>> > Thanks for reviewing Jon !
>> >
>> >
>>
>> Please wait a second.. I think the patch is incorrect.
>>
>> There is already code in myri10ge_rx_skb_build() which
>> attempts to set the truesize. However, it sets it to
>> the used, rather than the allocated size so it is apparently
>> incorrect.
>>
>> I'd prefer we fix that code.
>
> Well, I believe I did exactly that :)
>
> truesize of initial skb is fine.
>
> Then for everay frag added, you must add to skb-truesize the allocated
> memory for this frag.
>
> You add frags of a given size (small or big)
>
> In the end, its truesize += bytes * number_of_frags
>
> (bytes being small_size or big_size)
Right, I believe Eric's patch is correct, and I intend to apply it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:59 ` Eric Dumazet
2011-10-20 21:03 ` David Miller
@ 2011-10-20 21:04 ` Andrew Gallatin
2011-10-20 21:06 ` Eric Dumazet
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Gallatin @ 2011-10-20 21:04 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jon Mason, David Miller, netdev
On 10/20/11 16:59, Eric Dumazet wrote:
> Le jeudi 20 octobre 2011 à 16:45 -0400, Andrew Gallatin a écrit :
>> On 10/20/11 16:44, Eric Dumazet wrote:
>>> Le jeudi 20 octobre 2011 à 15:33 -0500, Jon Mason a écrit :
>>>> On Thu, Oct 20, 2011 at 3:10 PM, Eric Dumazet<eric.dumazet@gmail.com> wrote:
>>>>> skb->truesize must account for allocated memory, not the used part of
>>>>> it. Doing this work is important to avoid unexpected OOM situations.
>>>>>
>>>>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>>>>
>>>> Acked-by: Jon Mason<mason@myri.com>
>>>
>>> Thanks for reviewing Jon !
>>>
>>>
>>
>> Please wait a second.. I think the patch is incorrect.
>>
>> There is already code in myri10ge_rx_skb_build() which
>> attempts to set the truesize. However, it sets it to
>> the used, rather than the allocated size so it is apparently
>> incorrect.
>>
>> I'd prefer we fix that code.
>
> Well, I believe I did exactly that :)
>
> truesize of initial skb is fine.
>
> Then for everay frag added, you must add to skb-truesize the allocated
> memory for this frag.
>
> You add frags of a given size (small or big)
>
> In the end, its truesize += bytes * number_of_frags
>
> (bytes being small_size or big_size)
>
>
OK, I'm feeling foolish. I somehow missed the first hunk
of your patch (some local change made it not apply cleanly, and
I did not notice that).
Yes, your patch is correct. Sorry for the hassle!
Acked-by: Andrew Gallatin <gallatin@myri.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 21:04 ` Andrew Gallatin
@ 2011-10-20 21:06 ` Eric Dumazet
0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2011-10-20 21:06 UTC (permalink / raw)
To: Andrew Gallatin; +Cc: Jon Mason, David Miller, netdev
Le jeudi 20 octobre 2011 à 17:04 -0400, Andrew Gallatin a écrit :
> OK, I'm feeling foolish. I somehow missed the first hunk
> of your patch (some local change made it not apply cleanly, and
> I did not notice that).
>
> Yes, your patch is correct. Sorry for the hassle!
>
No worry, thanks a lot for reviewing !
>
> Acked-by: Andrew Gallatin <gallatin@myri.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] myri10ge: fix truesize underestimation
2011-10-20 20:10 [PATCH net-next] myri10ge: fix truesize underestimation Eric Dumazet
2011-10-20 20:33 ` Jon Mason
@ 2011-10-20 21:42 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2011-10-20 21:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, mason
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 20 Oct 2011 22:10:03 +0200
> skb->truesize must account for allocated memory, not the used part of
> it. Doing this work is important to avoid unexpected OOM situations.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-10-20 21:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-20 20:10 [PATCH net-next] myri10ge: fix truesize underestimation Eric Dumazet
2011-10-20 20:33 ` Jon Mason
2011-10-20 20:44 ` Eric Dumazet
2011-10-20 20:45 ` Andrew Gallatin
2011-10-20 20:59 ` Eric Dumazet
2011-10-20 21:03 ` David Miller
2011-10-20 21:04 ` Andrew Gallatin
2011-10-20 21:06 ` Eric Dumazet
2011-10-20 21:42 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.