On Wednesday 29 February 2012 16:47, Jack Morgenstein wrote: > Some comments. > > 1. Mr Cascardo's solution is only partial, and does not cover all the problem > cases. He > simply uncovered one of several examples of what lack-of-sync will do when > removing a device. > Mr. Cascardo found the kernel Oops that happens when a catastrophic error > occurs during device > removal. What if we receive a catas error while doing "init_one"? What if > we are in the middle > of catas error recovery (in the init_one stage), and we get a remove_one > request from higher up? > > There is a solution for this precise problem in the mthca driver > (infiniband/hw/mthca/mthca_main.c > infiniband/hw/mthca/mthca_catas.c). In the mthca driver, we DO in fact use > an "mthca_device_mutex" > for precisely the reason given in a. above. I see no reason not to do the > same thing here. > > This requires: > 1. mlx4_init_one(), mlx4_remove_one() and mlx4_restart_one all grab an > mlx4_device_mutex. > 2. new procedure __mlx4_remove_one(), which does not grab the mutex. > > Note that it is NOT enough to simply protect the removal operation. The > protection must wrap the > ENTIRE restart operation (both removal and init), because allowing a > remove in the middle of init_one > or restart_one would probably also cause a kernel Oops. > > 2. The intf_mutex is used with mlx4_un/register_device and > mlx4_un/register_interface. unregister_device is > used both in remove_one and in mlx4_change_port_types. I would hesitate > to grab that mutex for a more > global use. I think it is cleaner to add a device mutex > (mlx4_device_mutex) for initializing/removing/ > restarting the device. > > -Jack > Another thing -- what about the desired final state of the device? Say we do a remove one, and in the middle of this, we get a catas restart. The catas restart will wait until the remove-in-progress completes, and then will do its remove/init -- with the net result that the device is UP rather than DOWN.
This implies that we need some sort of state machine here. -Jack -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
