[ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow
Smith, Stan
stan.smith at intel.com
Thu Oct 7 12:18:37 PDT 2010
fair enough...
________________________________
From: Fab Tillier [mailto:ftillier at microsoft.com]
Sent: Thursday, October 07, 2010 11:04 AM
To: Smith, Stan; Tzachi Dar; ofw at lists.openfabrics.org
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: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Smith, Stan
Sent: Thursday, October 07, 2010 9:07 AM
To: Tzachi Dar; ofw at lists.openfabrics.org
Subject: Re: [ofw] [Patch][IPoIB_NIDS6_CM] Adding shutter_shut for Adapter Reset flow
________________________________
From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
Sent: Thursday, October 07, 2010 8:58 AM
To: Smith, Stan; ofw at lists.openfabrics.org
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:stan.smith at intel.com]
Sent: Thursday, October 07, 2010 5:50 PM
To: Tzachi Dar; ofw at lists.openfabrics.org
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: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Thursday, October 07, 2010 3:22 AM
To: Alex Naslednikov; ofw at lists.openfabrics.org
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: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Naslednikov
Sent: Wednesday, October 06, 2010 1:13 PM
To: ofw at lists.openfabrics.org
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20101007/3b783699/attachment.html>
More information about the ofw
mailing list