On Wed, 18 Nov 1998, Olaf Meyer wrote:

> Kaz and Alan,
> 
> thanks a lot!!! Your comments are VERY helpful :-) I think I found the
> bugs :-} I probably should invite you for a "virtual" beer some time :-)
> 
>  > You should set skb->free = 1 after allocating the buffer, otherwise it won't
>  > be freed properly by kfree_skb(). (At least in 2.0 kernels).
> 
> This was one of the problems! An obvious memory leak. When do I have to set
> the skb->free flag to 1? In the wavelan_packet_xmit routine dev_kfree_skb
> gets called without the free flag being set to 1. I guess the higher layer
> handles this, or is this a memory leak?

Yes, because it was the higher layer that manufactured the packet. My comments
had to do with packets that you manufacture yourself with alloc_skb().

Grep the /drivers/net directory for ''skb->free = 1'' and you can see how the
various drivers do this. Some of them have #ifdefs around it which test for the
kernel version number, so that they don't do it needlessly in kernels that
don't require it.

> If I use a skb for sending data, is the following sequence OK (with 2.0.*)?
> 
>   skb = alloc_skb(some_size, GFP_ATOMIC);
>   ...
>   ... call the transmit routine with (skb->data, skb->len)
>   ...
>   skb->free = 1;
>   kfree(skb, FREE_WRITE);

You can do it anywhere between the allocation and the freeing. If you had
many allocation places and one freeing place, I'd put it near the freeing place
and vice versa. :)

> Doing a save_flags/cli/restore_flags sequence in an interrupt doesn't hurt,
> does it?

If you know that the given code always runs with interrupts enabled, then you
can safely do cli()/sti(). 

Code that may sometimes be called with interrupts disabled and sometimes
with interrupts enabled should use the save_flags()/cli()/restore_flags()
so that it does not disturb the interrupt state.

You probably don't need to do this in your interrupt, since it's serialized
with respect to itself. Code that communicates with the interrupt should
disable interrupts when it needs to change the state of some objects that are
also accessed by the interrupt so that the interrupt sees either the stable
previous value or the new value, but not some half-baked value. Hence the
interrupt doesn't have to do anything special when accessing these shared
objects, since the interrupt simply won't happen when the data isn't in a safe
state.

The rule of thumb is do to cli()/sti() in any code that may race against an
interrupt, and to do start_bh_atomic()/end_bh_atomic() in any kernel code that
may race against bottom half processing (but not interrupts). This way you
allow interrupts to take place, just not bottom half processing.

-
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to [EMAIL PROTECTED]

Reply via email to