On Fri, Aug 12, 2016 at 07:54:03AM -0700, James Bottomley wrote:
> 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.
Sounds good, but for monday.
Nice weekend all.
--
Johannes Thumshirn Storage
jthumshirn@xxxxxxx +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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