[patch] dlinklist.h rewrite

Jeremy Allison jra at samba.org
Fri Feb 5 23:42:02 MST 2010


On Sat, Feb 06, 2010 at 05:01:15PM +1100, tridge at samba.org wrote:
> Hi Jeremy,
> 
> As we discussed earlier, I have rewritten the dlinklist.h macros to be
> O(1) for operations like DLIST_ADD_END()
> 
> The main patch is here:
> 
>   http://git.samba.org/?p=tridge/samba.git;a=commitdiff;h=3ba777d9b544cb3b781c0aa8d85847ddd959dba9
> 
> The original motivation was to fix a problem with large queues of
> messages in CTDBD, but I would like to apply it to Samba as well. We
> have over 100 places where we use DLIST_ADD_END() and they can all
> benefit a lot from moving to the O(1) system.
> 
> As explained in the new header, the approach is to change the linked
> list to use listhead->prev to point at the list tail. The tail still
> keeps its old value of listtail->next==NULL.
> 
> That means:
> 
>      1) with no entries in the list:
>           list_head == NULL
> 
>      2) with 1 entry in the list:
>           list_head->next == NULL
>           list_head->prev == list_head
> 
>      3) with 2 entries in the list:
>           list_head->next == element2
>           list_head->prev == element2
> 	  element2->prev == list_head
> 	  element2->next == NULL
> 
>      4) with N entries in the list:
>           list_head->next == element2
>           list_head->prev == elementN
> 	  elementN->prev == element{N-1}
> 	  elementN->next == NULL
> 
> This allows us to find the tail of the list by using list_head->prev,
> which means we can add to the end of the list in O(1) time.
> 
> The main change for users of the DLIST macros is that it is no longer
> safe to access el->prev directly. There were a few places in Samba
> that did access ->prev, and I have fixed those up with a series of
> followup patches in my dlist-rewrite branch here:
> 
>   http://git.samba.org/?p=tridge/samba.git;a=shortlog;h=refs/heads/dlist-rewrite
> 
> There was one use case of el->prev in Samba that went from O(1) to
> O(n) with this change, which was the cli_shutdown() code in
> libsmb/clientgen.c. The patch for that is here:
> 
>   http://git.samba.org/?p=tridge/samba.git;a=commitdiff;h=444077f1e9d3f08daf26769dc8f1e3342fefc359
> 
> The reason this was one tricky is that it is removing an element from
> a list without having a pointer to the list head. I added a new
> DLIST_HEAD() macro to make that case a bit easier to follow, but
> DLIST_HEAD() is O(n). I don't think it matters much for this use case
> as we don't expect to have thousands of DFS connections active in our
> client code at once. The benefit to all the other users of the
> DLIST_*() macros is worth this change I think.
> 
> The dlist-rewrite branch passes make test on my box for both Samba4
> and Samba3.

Wow - this looks really nice ! Impressive work !

Getting late here so I'll try and get to reviewing this
either over the weekend or (more likely) on Monday.

Cheers,

	Jeremy.


More information about the samba-technical mailing list