On Sat, 2012-01-14 at 12:46 +0000, Bart Van Assche wrote: > Only queue removal work after having changed the target state > into SRP_TARGET_REMOVED and not if that state was already equal > to SRP_TARGET_REMOVED. That allows to remove the state > SRP_TARGET_DEAD. Add a call to srp_disconnect_target() in > srp_remove_target() - due to previous changes it is now safe to > invoke that last function even if the IB connection has already > been disconnected. Rename srp_target_port.work into > srp_target_port.remove_work and move function srp_change_state() > to just before srp_change_state_to_removed().
Again, commit messages should be about why you changed, not a mechanical listing of what you change. "Rename srp_target_port.work into srp_target_port.remove_work" doesn't tell me anything. I had to read later patches to see that what you mean was really "Rename srp_target_port.work to reflect its usage and remove a future conflict when push the SCSI scan into it's own work task." This would have saved me some head scratching, and is why I try to review as much as I can of a series in one go -- all too often you make changes that leave me wondering until I see a later patch. I'm also not keen on how the code looks after these changes, but I do like getting rid of the DEAD state... I'm not sure there is a good way to handle the removal of the module that doesn't look a bit ugly. -- Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
