On Fri, 2016-08-12 at 16:39 +0200, Hannes Reinecke wrote:
> On 08/12/2016 04:34 PM, James Bottomley wrote:
> > On Fri, 2016-08-12 at 15:11 +0200, Johannes Thumshirn wrote:
> > > On Fri, Aug 12, 2016 at 12:08:54PM +0200, Johannes Thumshirn
> > > wrote:
> > > > Ok, we can't use the rphy because of wide-ports. We can't fix
> > > > it to an end device either, as this makes some peoples systems
> > > > unbootable. Now let's find a third option satisfying the needs
> > > > of SAS wide-ports and my customers (and others running 4.5+
> > > > with a SAS enclosure).
> > > >
> > > > I'm digging...
> > >
> > >
> > > To answer myself, Hannes suggested doing it like this:
> > >
> > > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> > > index 53ef1cb6..1d82053 100644
> > > --- a/drivers/scsi/ses.c
> > > +++ b/drivers/scsi/ses.c
> > > @@ -587,7 +587,7 @@ static void ses_match_to_enclosure(struct
> > > enclosure_device *edev,
> > >
> > > ses_enclosure_data_process(edev, to_scsi_device(edev
> > > ->edev.parent), 0);
> > >
> > > - if (is_sas_attached(sdev))
> > > + if (scsi_is_sas_rphy(&sdev->sdev_gendev))
> > > efd.addr = sas_get_address(sdev);
> > >
> > > if (efd.addr) {
> > >
> > >
> > > The reasoning behind this is, we only read the address if we have
> > > an actual sas_rphy.
> > >
> > > Would this be OK for you?
> >
> > Could you please debug the why? first before we start throwing
> > patches around. is_sas_attached(sdev) returns true if the sdev is
> > the child of a SAS controller. What is this thing you've found
> > that has a sdev attached to a SAS controller but isn't and end
> > device?
> >
> > if is_sas_attached() passes but scsi_is_sas_rphy() doesn't you've
> > got a device that is the child of a SAS host which has an rphy but
> > which isn't an expander or end device. That's pretty much the end
> > of the list of things which can lie at the end of rphys since we
> > lump the SATA possibilities in with end devices.
> >
> hpsa magic.
>
> The hpsa driver has some sdevs handled by the SAS transport class
> (for the pass-through devices) and some sdevs (eg logical volumes)
> which are not. As 'is_sas_attached' only checks if the _host_ has the
> SAS transport class attached (which it will have), it will not work
> as expected for devices which are not handled by the SAS transport
> class (like the 'normal' hpsa logical volumes). And the logical
> volumes don't even has a SAS address assigned to them, so in either
> case the original check will draw a blank here.
Thanks! I've found it in hpsa_add_device() for logical vs physical
setups and, indeed, the way they call scsi_add_device will ensure that
we don't attach the SAS transport class because the rphy isn't properly
initialised so the last check (rphy->identify.device_type ==
SAS_END_DEVICE) won't pass .... in fact it's a bit of a mess.
The change looks fine, since it's tighter than the original and, since
this will be the last consumer of is_sas_attached(), you can remove
that as well.
James
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html