Re: [PATCH net] amd/lance.c: dev_alloc_skb to netdev_alloc_skb

[Pradeep A. Dalvi]

On Thu, Jan 26, 2012 at 12:25 AM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> Le jeudi 26 janvier 2012 à 00:16 +0530, Pradeep A. Dalvi a écrit :
>> Replaced deprecating dev_alloc_skb with netdev_alloc_skb for amd/lance.c
>>   - Removed extra skb->dev = dev
>>
>> Signed-off-by: Pradeep A. Dalvi <netdev@xxxxxxxxxxxxxxxx>
>> ---
>>  drivers/net/ethernet/amd/lance.c |    9 ++++-----
>>  1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/lance.c b/drivers/net/ethernet/amd/lance.c
>> index a6e2e84..a8a6b45 100644
>> --- a/drivers/net/ethernet/amd/lance.c
>> +++ b/drivers/net/ethernet/amd/lance.c
>> @@ -871,12 +871,11 @@ lance_init_ring(struct net_device *dev, gfp_t gfp)
>>               struct sk_buff *skb;
>>               void *rx_buff;
>>
>> -             skb = alloc_skb(PKT_BUF_SZ, GFP_DMA | gfp);
>> +             skb = __netdev_alloc_skb(dev, PKT_BUF_SZ, GFP_DMA | gfp);
>
> Nack
>
> As I already explained, alloc_skb() and netdev_alloc_skb() are
> different.
>
> Why do you repost this ?
>
> nedev_alloc_skb() adds NET_SKB_PAD bytes.
>
> This driver doesnt need this extra space.
>
> (In fact it could use kmalloc(), since the skb themselves are not used
> at all : At RX time, we perform a copybreak)

Thanks Eric! Even if we try to skip NET_SKB_PAD i.e. max of 32 or
L1_CACHE_BYTES, SKB_DATA_ALIGN is anyway going to add some additional
bytes for the same reason.

And towards the end, as from my current understanding regarding skb
allocations, for MTU including alignments < 2048, there is not much to
worry about. As long as, it doesn't cross the boundary.

(And in fact, this driver doesn't even get compiled with current
configs, even after forcefully adding CONFIG_ISA & CONFIG_LANCE, of
course along with CONFIG_ISA_DMA_API. Had to change the Makefile)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/