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

[Phil Carns]

Whoops- sorry.  Wrong patch.  The previous one that I sent had a problem 
in it.  The attached one is corrected.

Phil Carns wrote:
> 
>> 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: 7014 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20071017/9a6bb3ad/pvfs2-bmi-addr-adjustment-0001.bin