This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: timeout.c in bsd_tcpip
Hi Andrew;
On Fri, May 09, 2003 at 09:41:25AM +0200, Andrew Lunn wrote:
>
> I get it now. _timeout is the array which is used for storage of the
> timeouts. timeout is a pointer and is the head of the linked list.
>
> do_timeout() currently walks the linked list starting from the first
> entry in the array. This is not necessarily the head of the linked
> list, eg the first entry may of expired. In this case, the subsequent
> entries are never looked at because the next pointer is NULL. Thats
> what the first hunk of the patch changes. It now walked from the head
> of the linked list.
>
> timeout() is responsible for adding new timeouts to the linked
> list. It should be looking at the array for empty slots. So using
> _timeout is correct. What is wrong is how it moves from slot to
> slot. The old code tries to walk a linked list. This is wrong. There
> is no linked list of empty slots. The second hunk changes this so that
> it simply tries each entry in the array in turn until it finds one
> which is not in use, or the end of the array is reached.
>
> The third hunk changes untimeout(). This removes a timeout. Again the
> old code walked a linked list starting from the first entry in the
> array and not the head of the list. You changed it to look at all
> elements in the array one by one. This will work, but maybe its more
> efficient to walk the linked list, but start from the head, ie
> timeout, not _timeout. What do you think?
I don't agree. I think that the old untimeout() is not only inefficient
but also wrong.
First, the link list headed by timeout may link many tcp timers
such as tt_rexmt, tt_persist, tt_keep, tt_2msl and tt_delack.
These timers are linked on the list by calling callout_reset() directly,
not calling timeout() since they don't need to allocate the entry space
for it. Actually the spaces for these timers are allocated
(or point to the allocated space) in tcp_newtcpcb(). Likewise these
timers are removed from the link list by calling callout_stop()
directly. On the other hand, untimeout() is used to remove the timer entry
which was allocated by timeout() previously.
Second, the old untimeout() change the link by calling callout_stop()
at the same time when it is tracing the link itself.
So I think that to look at all elements in the array(_timeout) is
not only efficient but also right.
>
> Apart from the minor point for the last hunk, im now happy with this
> patch. I've not actually run it, but i know understand it.
>
> Garry, what do you think? This is yours and Hugo's code. Ok to commit?
Motoya Kurotsu
Allied Telesis K.K.