On Sat, Dec 18, 2010 at 3:57 AM, Vasu Dev <[email protected]> wrote: > [reduced dist to only fcoe mail list] > > On Fri, 2010-12-17 at 21:17 +0800, Hillf Danton wrote: >> On Fri, Dec 17, 2010 at 4:05 AM, Vasu Dev <[email protected]> wrote: >> > On Thu, 2010-12-16 at 23:54 +0800, Hillf Danton wrote: >> >> In fc_lport_error(), FC_EX_CLOSED could be understood to be no more >> >> rettries, so resetting local port is reasonable and necessary. >> >> >> > >> > It is not reasonable/necessary in this case as FC_EX_CLOSED is issued on >> > lport reset thru exch block reset called there, so doing lport reset >> > again in same call flow is not needed unless I'm missing something to do >> > reset again here. Are hitting any bug which got fixed by this patch ? >> > >> Umm..why could lport reset then when no more retries, first? > > No more retries on FC_EX_CLOSED code and therefore no need to add reset > here as this patch does. The lport disable or destroy are cases to reset > the lport for its cleanup before disable or destroy and that case no > more retries. > >> And how about the state machine of lport then if nothing done, second? >> > > For instance fc_lport_destroy moves lport to disabled state before doing > exch_mgr_reset, so lport state is okay in that case by simply returning > on FC_EX_CLOSED error code.
Thanks for sharing the knowledge about resetting lport. Hillf > > >> It looks not buggy, I think. >> > >> >> When processing response to flogi request, FC_EX_CLOSED should be >> >> treated as other errors, since the state machine of local port could >> >> reach reset. >> >> >> >> Signed-off-by: Hillf Danton <[email protected]> >> >> --- >> >> >> >> --- a/drivers/scsi/libfc/fc_lport.c 2010-11-01 19:54:12.000000000 +0800 >> >> +++ b/drivers/scsi/libfc/fc_lport.c 2010-12-16 23:46:06.000000000 +0800 >> >> @@ -1013,7 +1013,7 @@ static void fc_lport_error(struct fc_lpo >> >> lport->retry_count); >> >> >> >> if (PTR_ERR(fp) == -FC_EX_CLOSED) >> >> - return; >> >> + goto reset; >> >> >> >> /* >> >> * Memory allocation failure, or the exchange timed out >> >> @@ -1029,6 +1029,7 @@ static void fc_lport_error(struct fc_lpo >> >> >> >> schedule_delayed_work(&lport->retry_work, delay); >> >> } else >> >> + reset: >> >> fc_lport_enter_reset(lport); >> >> } >> >> >> >> @@ -1428,9 +1429,6 @@ void fc_lport_flogi_resp(struct fc_seq * >> >> >> >> FC_LPORT_DBG(lport, "Received a FLOGI %s\n", fc_els_resp_type(fp)); >> >> >> >> - if (fp == ERR_PTR(-FC_EX_CLOSED)) >> >> - return; >> >> - >> > >> > Like other resp handlers in lport.c, above code is required instead just >> > removing for fc_lport_flogi_resp, this is to allow exch clean up on >> > exch block reset. >> First, this error could be processed correctly after lock. >> And it could be also processed with more cares not only in the above >> error handler. > > Not sure what is the point here, if locking is your concern here then > not sure how does removing above lines helps here. > >> >> And the state machine of lport is the major concern of the patch, >> simply returning >> helps little. > > Not sure what is that little help here given above all details. > > Vasu > > _______________________________________________ devel mailing list [email protected] https://lists.open-fcoe.org/mailman/listinfo/devel
