Please see comments below.
________________________________
From: [email protected]
[mailto:[email protected]] On Behalf Of Alex Naslednikov
Sent: Wednesday, October 06, 2010 4:13 AM
To: [email protected]
Subject: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset
flow
The patch below prevents race with BSOD when NDIS call receive callback of
ipoib while
some ipoib global objects are under destruction (like p_adapter ->p_port).
It was tested by night regression run on IPoIB driver
---
Adding shutter_shut for Adapter Reset flow
Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)
Index: kernel/ipoib_adapter.cpp
===================================================================
--- kernel/ipoib_adapter.cpp (revision 2938)
+++ kernel/ipoib_adapter.cpp (working copy)
@@ -988,10 +988,13 @@
p_adapter->reset = TRUE;
if( p_adapter->h_pnp )
- {
+ {
+ Why the extra blank line? Please remove.
h_pnp = p_adapter->h_pnp;
p_adapter->h_pnp = NULL;
status = p_adapter->p_ifc->dereg_pnp( h_pnp,
__ipoib_pnp_dereg );
+ // Wait until NDIS will return all indicated NBLs
that were received
+ shutter_shut( &p_adapter->recv_shutter );
I'm a bit concerned in that shutter_shut( &p_adapter->recv_shutter) could be
called twice as 'ipoib_shutdown_ex()' also calls
shutter_shut( &p_adapter->recv_shutter) and then the PNP callback would also
call shutter_shut( &p_adapter->recv_shutter) ?
I do not see a checked build assert() from shutter.h for shutter_shut() being
called on the same shutter, so it must be OK?
Thoughts?
Otherwise the patch looks OK and tests OK.
if( status == IB_SUCCESS )
status = IB_NOT_DONE;
}
Index: kernel/ipoib_port.cpp
===================================================================
--- kernel/ipoib_port.cpp (revision 2946)
+++ kernel/ipoib_port.cpp (working copy)
@@ -830,11 +830,15 @@
IPOIB_PRINT( TRACE_LEVEL_ERROR, IPOIB_DBG_OBJ,
("ref type %d ref_cnt %d\n", ref_init,
p_port->obj.ref_cnt) );
#endif
- // The port is started as paused and NDIS calls latter to
ipoib_restart. We
- // shut the recv_shuter for now and alive it on ipoib_restart.
We did
- // it in this way since MpInitializeInternal also calls in reset
and than
- // we need to set the rec ref count to 1 //TODO !!!!!!!!!!1
- //
+ /* The steps of the initialization are as depicted below:
+ I. adapter_init() calls shutter_init(), the shutter
counter is set to 0
+ II. ipoib_pnp_cb() calls to ipoib_port_init() that
calls to __port_init() that SHOULD set shutter counter to -MAX_OPERATIONS
+ That is, the code below should call to
shutter_shut()
+ III.NDIS calls to ipoib_restart() that calls to shutter_alive.
Now, shutter_counter is again 0.
+ IV. NDIS call to ipoib_pause() that sets the adapter to
pause state, calls to shutter_shut.
+ Now, the counter is again equals to
-MAX_OPERATIONS
+ V. NDIS calls to ipoib_restart that calls to
shutter_alive. Shutter counter is 0 and we can start working
+ */
if ( p_adapter->ipoib_state == IPOIB_INIT) {
IPOIB_PRINT( TRACE_LEVEL_INFORMATION,
IPOIB_DBG_RECV,
Alexander (XaleX) Naslednikov
SW Networking Team
Mellanox Technologies
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
