Re: [PATCH] mlx4: prevent the device from being removed concurrently

Mr Cascardo,

If this is at a customer site, there is a workaround available for you.

In file /etc/modprobe.d/mlx4_core.conf, enter a line:
options mlx4_core internal_err_reset=0

(do "modinfo mlx4_core" to see a description of the module parameter).

Setting this parameter to 0 in the modprobe conf file will cause the driver to simply report the
internal error failure (in /var/log/messages), but the device will not be restarted.

This will avoid your Oops -- the EEH flow will then be the only flow restarting the device.

(Note that even if you implement my suggestion, you will still have a situation where the device
will be reset TWICE).

Again, this is a workaround for current users.  The issue needs to be fixed so that even when the
internal error does cause a reset, the kernel will not crash.

Unfortunately, I do not have the time now to generate a patch.

-Jack

On Wednesday 29 February 2012 17:19, Jack Morgenstein wrote:

On Wednesday 29 February 2012 16:47, Jack Morgenstein wrote:

quoted

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html