On 09/04/2014 02:49 AM, Erez Shitrit wrote:
Hi Doug,
The idea that "big" ipoib workqueue is only for the IB_EVENT_XXX looks
good to me.
one comment here: I don't see where the driver flushes it at all.
IMHO, when the driver goes down you should cancel all pending tasks over
that global wq prior the call for the destroy_workqueue.
I understand your thought, but I disagree with your conclusion. If we
ever get to the portion of the driver down sequence where we are
removing the big flush work queue and there are still items on that
work queue, then we have already failed and we are going to crash
because there are outstanding flushes for a deleted device.
it is not a rare case, sm events (pkey, hand-over, etc.) while driver
restart on devices in the cluster, and you are there.
removing one device of few on the same host, it is a rare condition,
anyway i agree that cancel_work is not the solution.
The real issue here is that we need to make sure it's not possible to
delete a device that has outstanding flushes, and not possible to
flush a device in the process of being deleted (Wendy, don't get mad
at me, but the rtnl lock jumps out as appropriate for this purpose).
we can remove the device and drain the workqueue while checking on each
event that this event doesn't belong to deleted device (we have the list
of all existing priv objects in the system via ib_get_client_data) after
that we can be sure that no more works are for that deleted device.
The current logic calls the destroy_workqueue after the remove_one, i
think you should cancel the pending works after the
ib_unregister_event_handler call in the ipoib_remove_one function,
The ib_remove_one is device specific while the big flush work queue is
not. As such, that can cancel work for devices other than the device
being removed with no clear means of getting it back.
otherwise if there are pending tasks in that queue they will schedule
after the dev/priv are gone.
I agree that there is a problem here. I think what needs to happen is
that now that we have a work queue per device, and all flushes come in
to us via a work queue that does not belong to the device, it is now
possible to handle flushes synchronously. This would allow us to lock
around the flush itself and prevent removal until after the flush is
complete without getting into the hairy rat hole that is trying to
flush the flush workqueue that might result in either flushing or
canceling the very device we are working on.
--
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
