e1000 driver 2.6.18 - how to waste processor cycles

e1000 driver 2.6.18 - how to waste processor cycles

[Jeff V. Merkey]

Is there a good reason the skb refill routine in e1000_alloc_rx_buffers 
needs to go and touch and remap skb memory
on already loaded descriptors/  This seems extremely wasteful of 
processor cycles when refilling the ring buffer.

I note that the archtiecture has changed and is recycling buffers from 
the rx_irq routine and when the routine is called
to refill the ring buffers, a lot of wasteful and needless calls for 
map_skb is occurring.

Jeff

Re: e1000 driver 2.6.18 - how to waste processor cycles

[Jesse Brandeburg]

included netdev...

On 11/8/06, Jeff V. Merkey <jmerkey@soleranetworks.com> wrote:
>
> Is there a good reason the skb refill routine in e1000_alloc_rx_buffers
> needs to go and touch and remap skb memory
> on already loaded descriptors/  This seems extremely wasteful of
> processor cycles when refilling the ring buffer.
>
> I note that the archtiecture has changed and is recycling buffers from
> the rx_irq routine and when the routine is called
> to refill the ring buffers, a lot of wasteful and needless calls for
> map_skb is occurring.

we have to unmap the descriptor (or at least do
pci_dma_sync_single_for_cpu / pci_dma_sync_single_for_device) because
the dma API says we can't be guaranteed the cacheable memory is
consistent until we do one of the afore mentioned pci dma ops.

we have to do *something* before we access it.  Simplest path is to
unmap it and then recycle/map it.

If you can show that it is faster to use pci_dma_sync_single_for_cpu
and friends I'd be glad to take a patch.

Hope this helps,
  Jesse

Re: e1000 driver 2.6.18 - how to waste processor cycles

[David Miller]

From: "Jesse Brandeburg" <jesse.brandeburg@gmail.com>
Date: Wed, 8 Nov 2006 17:01:44 -0800

> If you can show that it is faster to use pci_dma_sync_single_for_cpu
> and friends I'd be glad to take a patch.

The problem is if you don't recycle the buffer and really unmap,
you'll flush twice.  That can potentially be expensive.

Re: e1000 driver 2.6.18 - how to waste processor cycles

[Jeffrey V. Merkey]

Jesse Brandeburg wrote:

> included netdev...
>
> On 11/8/06, Jeff V. Merkey <jmerkey@soleranetworks.com> wrote:
>
>>
>> Is there a good reason the skb refill routine in e1000_alloc_rx_buffers
>> needs to go and touch and remap skb memory
>> on already loaded descriptors/  This seems extremely wasteful of
>> processor cycles when refilling the ring buffer.
>>
>> I note that the archtiecture has changed and is recycling buffers from
>> the rx_irq routine and when the routine is called
>> to refill the ring buffers, a lot of wasteful and needless calls for
>> map_skb is occurring.
>
>
> we have to unmap the descriptor (or at least do
> pci_dma_sync_single_for_cpu / pci_dma_sync_single_for_device) because
> the dma API says we can't be guaranteed the cacheable memory is
> consistent until we do one of the afore mentioned pci dma ops.

In the case I am referring to, the memory is already mapped with a 
previous call, which means it may be getting
mapped twice.

Jeff

>
> we have to do *something* before we access it.  Simplest path is to
> unmap it and then recycle/map it.
>
> If you can show that it is faster to use pci_dma_sync_single_for_cpu
> and friends I'd be glad to take a patch.
>
> Hope this helps,
>  Jesse
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: e1000 driver 2.6.18 - how to waste processor cycles

[Jesse Brandeburg]

On 11/9/06, Jeffrey V. Merkey <jmerkey@soleranetworks.com> wrote:
> In the case I am referring to, the memory is already mapped with a
> previous call, which means it may be getting
> mapped twice.

I guess maybe I'm not keeping up with you.  This is what I see looking
in 2.6.18, i see e1000_clean_rx_irq:

check done bit
pci_unmap_single
copybreak and recycle
OR
hand buffer up stack

the only branch before the unmap is the napi break out, and in that
case we don't change any memory state, so alloc will not do anything.

As for alloc rx, we always map, because we always unmapped.

Did I miss something?  I would appreciate a more detailed explanation
of what you see going wrong.

Re: e1000 driver 2.6.18 - how to waste processor cycles

[Jeff V. Merkey]

Jesse Brandeburg wrote:

> On 11/9/06, Jeffrey V. Merkey <jmerkey@soleranetworks.com> wrote:
>
>> In the case I am referring to, the memory is already mapped with a
>> previous call, which means it may be getting
>> mapped twice.
>
>
> I guess maybe I'm not keeping up with you.  This is what I see looking
> in 2.6.18, i see e1000_clean_rx_irq:


Check e1000_alloc_rx_buffers:

if (skb already exists in ring buffer)
  goto map_skb:
else
   dev_alloc_skb
  ( drop through to map_skb)


map_skb:
  pci_map_single

Jeff

>
> check done bit
> pci_unmap_single
> copybreak and recycle
> OR
> hand buffer up stack
>
> the only branch before the unmap is the napi break out, and in that
> case we don't change any memory state, so alloc will not do anything.
>
> As for alloc rx, we always map, because we always unmapped.
>
> Did I miss something?  I would appreciate a more detailed explanation
> of what you see going wrong.
>
>

Re: e1000 driver 2.6.18 - how to waste processor cycles

[Jeff V. Merkey]

Jeff V. Merkey wrote:

> Jesse Brandeburg wrote:
>
>> On 11/9/06, Jeffrey V. Merkey <jmerkey@soleranetworks.com> wrote:
>>
>>> In the case I am referring to, the memory is already mapped with a
>>> previous call, which means it may be getting
>>> mapped twice.
>>
>>
>>
>> I guess maybe I'm not keeping up with you.  This is what I see looking
>> in 2.6.18, i see e1000_clean_rx_irq:
>
>
>
> Check e1000_alloc_rx_buffers:
>
> if (skb already exists in ring buffer)
>  goto map_skb:
> else
>   dev_alloc_skb
>  ( drop through to map_skb)
>
>
> map_skb:
>  pci_map_single
>
> Jeff


>
>>
>> check done bit
>> pci_unmap_single
>> copybreak and recycle
>> OR
>> hand buffer up stack
>>
>> the only branch before the unmap is the napi break out, and in that
>> case we don't change any memory state, so alloc will not do anything.
>>
>> As for alloc rx, we always map, because we always unmapped.
>
Unmapping every single buffer in rx_irq the remapping them in 
alloc_rx_buffers is wasteful of cycles. 

Jeff

>>
>> Did I miss something?  I would appreciate a more detailed explanation
>> of what you see going wrong.
>>
>>
>
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>