> Quoting Yosef Etigin <[EMAIL PROTECTED]>: > Subject: Re: [PATCHv2 1/2] ipoib: handle pkey change events > > Michael S. Tsirkin wrote: > >>Quoting Yosef Etigin <[EMAIL PROTECTED]>: > >>Subject: Re: [PATCHv2 1/2] ipoib: handle pkey change events > >> > >>Michael S. Tsirkin wrote: > >> > >>>>Quoting Yosef Etigin <[EMAIL PROTECTED]>: > >>>>Subject: Re: [PATCHv2 1/2] ipoib: handle pkey change events > >>>> > >>>>Michael S. Tsirkin wrote: > >>>> > >>>> > >>>>>>Quoting Yosef Etigin <[EMAIL PROTECTED]>: > >>>>>>Subject: [PATCHv2 1/2] ipoib: handle pkey change events > >>>>>> > >>>>>>This issue was found during partitioning & SM fail over testing. > >>>>>> > >>>>>>* Added flush flag to ipoib_ib_dev_stop(), ipoib_ib_dev_down() alike > >>>>>>* Rename the polling thread work to 'pkey_poll_task' to avoid ambiguity > >>>>>>* Upon PKEY_CHANGE event, schedule a work that restarts the QP > >>>>>>* Restart child interfaces before parent > >>>>> > >>>>> > >>>>>What's the reason for this change? > >>>>>Is this a separate bugfix? > >>>>>You might want to put this info in the log. > >>>>> > >>>> > >>>>The reason is that if the child are restarted after the parent, and the > >>>>parent is > >>>>not up, then the flush function returns immediately due to the > >>>>INITIALLIZED bit test. > >>>>Now I think that we might use a goto statement instead. > >>> > >>> > >>>So ... what the problem? I still don't see it. > >>> > >> > >>If I get pkey change event, I want to restart all active ifaces on my port. > >>If > >>the parent is not marked with IPOIB_FLAG_INITIALIZED, the function returns > >>before it has a chance to recursively restart child ifaces. > > > > > > So now, if restart_qp is set, you are going to open it, and it's not > > initialized? > > > > BTW, if the interface is not initialized, is not QP in reset already? > > So can't we just move the code that assign the pkey to the open call? > > > No - I'm going to open its *child* interface. The problem is that parents > "mask out" > their children.
Aha, I see this now. You might want to explain this in the comment. Something like: /* Flush any child interfaces too - * they might be up even if the parent is down */ > > Another idea - won't it be cleaner to have a function ipoib_restart_qp > > (functionally similiar to ib_dev_flush, but also changing the QP) > > than adding a flag to ib_dev_flush? > > > It might be, but we wanted to avoid code duplication (the only difference is > 2-3 lines) OK, it's a valid approach too. -- MST _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
