[Pvfs2-developers] Re: halloween bug (possible patch update)
pcarns at wastedcycles.org
Wed Oct 17 11:41:24 EDT 2007
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 7014 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20071017/9a6bb3ad/pvfs2-bmi-addr-adjustment-0001.bin
More information about the Pvfs2-developers