[Pvfs2-developers] halloween bug (possible patch update)

[Phil Carns]

> Actually, one more follow up on this specific patch to wrap up.  The 
> problem that I suspected actually does not occur, although it may have 
> been by luck :)  There is a tcp_addr_data->bmi_addr field that gets used 
> as an argument to the forget_callback() function.  That bmi_addr field 
> is set to zero unless it is filled in by the addr_reg_callback() 
> function.  That means that on the client side, the forget_callback() 
> function actually just fails because it searches for a BMI_addr_t of 
> value 0 in the reference list.
> 
> I just tested it out, and pvfs2-client-core was able to recover from a 
> network error just fine with the patch in place.

I still agree that it would probably be better in the long run to 
eliminate the bmi ref list, but I kept looking at the current situation 
to get a fix in the meantime.  The attached patch builds on what is 
currently in trunk with a few changes:

- removes additional ref list lock that was added to 
addr_list_callback() with one of the recent commits.  I think this 
cleanup was not directly related to the halloween bug.  It appears 
reasonable, but it introduced a lock ordering problem between the ref 
list mutex and bmi_tcp's interface mutex.
- The new forget_callback() function had three possible issues: the same 
lock ordering issue as in the above bullet (although the recent commit 
to release the interface mutex may have also fixed it), the possibility 
of deleting an address with a non-zero reference count, and also I think 
in bmi_tcp it didn't take care of freeing bmi_tcp's internal method 
address.  I changed the forget_callback implementation to just mark the 
target addresses in a queue for the bmi control layer to look at later 
rather than trying to immediately free the address.  I think this fixes 
all three problems by having the control layer be the component that 
drives the address release from the top down (using the same mechanism 
already in place for when the reference count reaches zero).
- moved the code in the set_info() function for dropping a bmi address 
into a subroutine so that the same code path is used for both the 
forget_callback() and the set_info(BMI_DROP_ADDR) case.

Basically I think we already had a code path that did the right thing 
(in terms of bmi_tcp) in place to get triggered when a bmi addr 
reference count hit zero.  The problem in this slowdown situation, 
though, was that the socket was being closed _after_ the ref count hit 
zero.  There was never any activity on the addr after that to trigger 
the cleanup.  The forget_callback() essentially is now giving methods a 
way to tell the control layer to consider dropping the address again 
independent of what the reference count has done.

I'm still testing on this some, but I wanted to go ahead and throw it 
out there for discussion.

-Phil
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pvfs2-bmi-addr-adjustment.patch
Type: text/x-patch
Size: 6794 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20071017/55914ec3/pvfs2-bmi-addr-adjustment.bin