fair enough... ________________________________ From: Fab Tillier [mailto:[email protected]] Sent: Thursday, October 07, 2010 11:04 AM To: Smith, Stan; Tzachi Dar; [email protected] Subject: RE: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow
If you are going to include a change in RC4, I think it would be good to check in the associated code - that way the RC4 code is available in SVN somewhere. If the patch ends up causing issues, it can be rolled back or fixed at that point. This helps users that might build a tweaked version of what they think is RC4, without missing some potentially important change. -Fab From: [email protected] [mailto:[email protected]] On Behalf Of Smith, Stan Sent: Thursday, October 07, 2010 9:07 AM To: Tzachi Dar; [email protected] Subject: Re: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow ________________________________ From: Tzachi Dar [mailto:[email protected]] Sent: Thursday, October 07, 2010 8:58 AM To: Smith, Stan; [email protected] Subject: RE: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow I'm fine with the ipoib forwarding checkin. (if no one else has objection). OK, will make it part of winOFED 2.3 RC4; meaning I will go ahead and commit the patch to winOFED SVN. As for the shutter checkin, we have been running with it for a few days, but I don't know if shutdown is really tested. Will include this patch in 2.3 RC4 as I have not witnessed any shutdown problems so far in pre-RC4 testing. I'll hold back on committing the patch until we have more shutdown testing on both sides of the pond. thanks, stan. Thanks Tzachi From: Smith, Stan [mailto:[email protected]] Sent: Thursday, October 07, 2010 5:50 PM To: Tzachi Dar; [email protected] Subject: RE: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow Hello, If shutter_shut() is called twice an ASSERT() in shutter.h::shutter_shut() will fire. I do not see the ASSERT() fire so we are OK; although code inspection looked as if shutter_shut() 'could' be called twice: 1) ex_shutdown path - power down case 2) PNP port down path What are your plans for SVN committing this patch and the patch for fixing forwarding of ipoib ? ________________________________ From: [email protected] [mailto:[email protected]] On Behalf Of Tzachi Dar Sent: Thursday, October 07, 2010 3:22 AM To: Alex Naslednikov; [email protected] Subject: Re: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow Hi Stan, Shuter_shut should not be called twice. In the case that it will than it will probably stay shut "for ever". The real question is whether it can be called twice. I believe that there is an assumption in the code that once reset is called shut down cannot be called, so the shutter will not be called twice. Thanks Tzachi From: [email protected] [mailto:[email protected]] On Behalf Of Alex Naslednikov Sent: Wednesday, October 06, 2010 1:13 PM 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 ) - { + { + 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 ); 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
