On 05/24/2013 01:10 AM, Dan Carpenter wrote:
On Fri, May 24, 2013 at 12:57:17AM +0300, Xenia Ragiadakou wrote:
On 05/24/2013 12:18 AM, Dan Carpenter wrote:
[snip]
I don't know much about networking. The problems with this patch
are:
1) No reason to apply it in the changelog.
2) The kfree_skb() wasn't updated to match. After sending my email
I saw that you fixed this in a later patch and I sent a grumpier
than necessary email. Sorry. Splitting that up into two patches
breaks git bisect so it's not allowed. Anyways, it's a newbie
mistake. No worries again.
3) The comments on dev_alloc_skb() say it's obsolete and we should
use netdev_alloc_skb() instead.
I don't actually know when it's appropriate to use one or the other
honestly... It could be that you're right about this here. I
honestly don't know, so that's why it has to be explained to me in
the changelog.
regards,
dan carpenter
You are right, since there is a net_device pointer in the function
there is no reason not to put netdev_alloc_skb().
I will fix this patch and i will try to justify it in changelog (as much as
my comprehension and my english permit it).
Also i will make all the changes in one patch, since i should not
separate function pairs I agree.
Yep. That's fine. All four of these patches can be merged into one
and it still fits in the one change per patch rule.
regards,
dan carpenter
Hi Dan,
I looked more carefully at the definition of the netdev_alloc_skb()
(and dev_alloc_skb()) and it is meant to be used for the allocation
of socket buffers in the case of packet reception and not for
the allocation of socket buffers that will be used for transmission.
netdev_alloc_skb() allocates extra bytes padded to the headroom
(NET_SKB_PAD), maybe for use by the higher layers in the network
stack (i am not sure). On the transmit path there is no need
for such padding since the headers are already well specified by
the upper layers.
Another difference between the two functions, is that netdev_alloc_skb()
for sizes smaller than the page size, allocates directly a page fragment of
equal size. That maybe is an enhancement for reducing memory fragmentation
but again i am not sure.
So when i read that "sk_buff allocation is done with alloc_skb() or
dev_alloc_skb(); drivers use dev_alloc_skb()", it was probably
refering to skbs on the receive path.
However, the changes that i made in my patch targeted allocations
of buffers used for the transmission of packets, so they are not justified.
I will send a revert patch to correct my last changes on ozwpan
for those that already applied it.
Thank you Dan for your remarks that made me realize my mistake
before being too late. Sorry about that, i will be more careful in the
future and i will provide justification in my changelogs.
regards,
Ksenia
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
