[Open-FCoE] [RFC PATCH 02/28] libfc:Add a callback to the fc_lport for fc_rport events

Joe Eykholt jre at nuovasystems.com
Fri Sep 12 06:07:20 UTC 2008


Robert Love wrote:
> We want to know when an rport has successfully logged in
> or not. The lport needs to know if the NS's rport is READY
> or not. If it's not READY then the lport needs to decide
> what it wants to do. The rport and NS shouldn't be
> making decisions for the lport.
> 
> This patch creates a callback function for the lport and calls
> it when the rport is READY.
> 
> The lport will call dns_register when it sees that the NS rport
> is READY.
> 
> Signed-off-by: Robert Love <robert.w.love at intel.com>
> ---
> 
>  drivers/scsi/libfc/fc_lport.c |   22 ++++++++++++++++++++++
>  drivers/scsi/libfc/fc_rport.c |    8 +-------
>  include/scsi/libfc/libfc.h    |    8 ++++++++
>  3 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
> index b390a32..7dc09f7 100644
> --- a/drivers/scsi/libfc/fc_lport.c
> +++ b/drivers/scsi/libfc/fc_lport.c
> @@ -58,6 +58,25 @@ static int fc_frame_drop(struct fc_lport *lp, struct fc_frame *fp)
>  	return 0;
>  }
>  
> +static int fc_lport_rport_event(struct fc_lport *lport, struct fc_rport *rport,
> +				enum fc_lport_event event)
> +{
> +	fc_lport_lock(lport);
> +	if (rport->port_id == FC_FID_DIR_SERV) {
> +		switch (event) {
> +		case LPORT_EV_RPORT_CREATED:
> +			lport->tt.dns_register(lport);
> +			break;
> +		case LPORT_EV_RPORT_FAILED:
> +			fc_lport_enter_reset(lport);
> +			break;
> +		}
> +	}
> +	fc_lport_unlock(lport);
> +
> +	return 0;
> +}

This could do the rport_id check before grabbing the lport lock.

> +
>  static const char *fc_lport_state(struct fc_lport *lp)
>  {
>  	const char *cp;
> @@ -921,6 +940,9 @@ int fc_lport_init(struct fc_lport *lp)
>  	if (!lp->tt.lport_logout)
>  		lp->tt.lport_logout = fc_lport_logout;
>  
> +	if (!lp->tt.lport_event)
> +		lp->tt.lport_event = fc_lport_rport_event;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(fc_lport_init);
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index c7360db..012200b 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -340,13 +340,7 @@ static void fc_rport_enter_ready(struct fc_rport *rport)
>  	if (fc_rp_debug)
>  		FC_DBG("remote %6x ready\n", rport->port_id);
>  
> -	if (rport == lport->dns_rp &&
> -	    lport->state == LPORT_ST_DNS) {
> -		fc_lport_lock(lport);
> -		del_timer(&lport->state_timer);
> -		lport->tt.dns_register(lport);
> -		fc_lport_unlock(lport);
> -	}
> +	lport->tt.lport_event(lport, rport, LPORT_EV_RPORT_CREATED);
>  }

I think it's safe for the rport code to do the callback without
holding the rport lock (check that), and that would help fix the lock
dependency problems.  Although that means doing the callback
outside of the rport_enter_* functions, and might be awkward.
One way to achieve this is to have an indication in the rport that
tells fc_rport_unlock() to do the callback after clearing the
flag and dropping the lock.

In case this isn't obvious, what I'm talking about is:  add a
field in fc_rport_libfc_priv called event.  Instead of calling the
event handler above, it would merely do (I didn't check all the
names, but you'll get the idea):

	rdata->event = LPORT_EV_RPORT_CREATED;

Then in unlock:

+	event = rdata->event;
+	rdata->event = LPORT_EV_RPORT_NONE;
	spin_unlock_bh(&rdata->rp_lock);
+	if (event != LPORT_EV_RPORT_NONE)
+		lport->tt.lport_event(lport, rport, event);

>  
>  static void fc_rport_enter_start(struct fc_rport *rport)
> diff --git a/include/scsi/libfc/libfc.h b/include/scsi/libfc/libfc.h
> index b139aed..782c862 100644
> --- a/include/scsi/libfc/libfc.h
> +++ b/include/scsi/libfc/libfc.h
> @@ -100,6 +100,11 @@ enum fc_lport_state {
>  	LPORT_ST_RESET
>  };
>  
> +enum fc_lport_event {
> +	LPORT_EV_RPORT_CREATED = 0,
> +	LPORT_EV_RPORT_FAILED
> +};
> +
>  enum fc_rport_state {
>  	RPORT_ST_NONE = 0,
>  	RPORT_ST_INIT,		/* initialized */
> @@ -320,6 +325,9 @@ struct libfc_function_template {
>  	int (*lport_reset)(struct fc_lport *);
>  	int (*lport_logout)(struct fc_lport *);
>  
> +	int (*lport_event)(struct fc_lport *, struct fc_rport *,
> +			   enum fc_lport_event);
> +
>  	/**
>  	 * Remote Port interfaces
>  	 */
> 
> _______________________________________________
> devel mailing list
> devel at open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel




More information about the devel mailing list