On Thu, 2008-09-11 at 23:07 -0700, Joe Eykholt wrote:
> 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 <[EMAIL PROTECTED]>
> > ---
> >
> > 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.
Yeah, my problem with the solution below is that without the rport lock
held the following can happen.
event = rdata->event;
rdata->event = LPORT_EV_RPORT_NONE;
spin_unlock_bh(&rdata->rp_lock);
*** RESET ***
if (event != LPORT_EV_RPORT_NONE)
lport->tt.lport_event(lport, rport, event);
That would create a race as to whether the rport object would be freed
before or after the lport event handler checks the FID. I guess I don't
need to pass an rport pointer, I can just pass the FID.
> 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
> > [email protected]
> > http://www.open-fcoe.org/mailman/listinfo/devel
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://www.open-fcoe.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
