On Thu, 2009-07-09 at 10:12 +0800, tom wrote:
> On Wed, 2009-07-01 at 20:37 +0800, jack wang wrote:
> > >From 24abd7f05d2293e03aacbdd9d620d6378ea5e2aa Mon Sep 17 00:00:00 2001
> > From: Tom Peng <tom_peng@xxxxxxxxx>
> > Date: Wed, 1 Jul 2009 19:04:01 +0800
> > Subject: [PATCH 3/5] reuse the original port when reenable the phy in a
> port
> >
> > Signed-off-by: Tom Peng <tom_peng@xxxxxxxxx>
> > Signed-off-by: Jack Wang <jack_wang@xxxxxxxxx>
> > Signed-off-by: Lindar Liu <lindar_liu@xxxxxxxxx>
> > ---
> > drivers/scsi/libsas/sas_port.c | 19 +++++++++++++++----
> > 1 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/libsas/sas_port.c
> b/drivers/scsi/libsas/sas_port.c
> > index e6ac59c..5aad165 100644
> > --- a/drivers/scsi/libsas/sas_port.c
> > +++ b/drivers/scsi/libsas/sas_port.c
> > @@ -69,16 +69,27 @@ static void sas_form_port(struct asd_sas_phy *phy)
> > SAS_DPRINTK("phy%d matched wide port%d\n", phy->id,
> > port->id);
> > break;
> > - } else if (*(u64 *) port->sas_addr == 0 &&
> > port->num_phys==0) {
> > - memcpy(port->sas_addr, phy->sas_addr,
> > SAS_ADDR_SIZE);
> > - break;
> > }
> > spin_unlock(&port->phy_list_lock);
> > }
> > + /* The phy does not match any port currently existed, create new one
> > */
> > + if (i == sas_ha->num_phys) {
> > + for (i = 0; i < sas_ha->num_phys; i++) {
> > + port = sas_ha->sas_port[i];
> > + spin_lock(&port->phy_list_lock);
> > + if (*(u64 *)port->sas_addr == 0
> > + && port->num_phys == 0) {
> > + memcpy(port->sas_addr, phy->sas_addr,
> > + SAS_ADDR_SIZE);
> > + break;
> > + }
> > + spin_unlock(&port->phy_list_lock);
> > + }
> > + }
>
> I don't understand why you need this. It seems identical to the else
> leg of the if in the for loop above it, so it wouldn't seem to add
> anything to the code.
>
> James
>
>
> --
>
> Hi James,
> Just think one situation: a HBA has two wide ports, port 0 (phy0, phy1,
> phy2, phy3) attached to expander A, port1 (phy4, phy5, phy6, phy7) attached
> to expander B, supposed the cable of port0 was plugged out, port1 still
> here. After a while, for some reason phy4(or phy5 phy6 phy7) linked down
> then linked up, the code will added this phy to form a new port, it is
> port0, rather than added this phy to port1.
>
> I have tested this bug, use SMP utility phy_control to disable phy4
> (expander B)and then enabled it, it formed new port: port0.
>
> To fix this, we can detect all of the ports existed and if no any port
> matched, we form new one.
OK, get it now, sorry ... missed the else clause removal in the patch.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
