RE: Issue in sas_ex_discover_dev() for multiple level of SAS expanders in a domain

Li, Eric (Honggang)
2024-05-08T03:18:26Z

Internal Use - Confidential
> -----Original Message-----
> From: John Garry <john.g.garry@oracle.com>
> Sent: Tuesday, May 7, 2024 11:15 PM
> To: Li, Eric (Honggang) <Eric.H.Li@Dell.com>; Jason Yan <yanaijie@huawei.com>;
> james.bottomley@hansenpartnership.com; Martin K . Petersen <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> Subject: Re: Issue in sas_ex_discover_dev() for multiple level of SAS expanders in a domain
>
>
> [EXTERNAL EMAIL]
>
> On 07/05/2024 12:17, Li, Eric (Honggang) wrote:
> >> Sure, I don't particularly like it as a fix either. But first I would
> >> like to get your setup working again to verify that only this needs
> >> fixing. Indeed something else may be broken since a1b6fb947f923. In
> >> addition, if we need to backport a fix, I would only like to backport a fix for real known
> broken topologies, and not theoretical issues not experienced.
> >>
> >> But what exact setup do you have? I am confused, as you seem to be
> >> talking about your setup being broken, but then other setup may also
> >> being broken, but you don't have access to another setup. What is the other setup?
> >>
> > This issue was reported by our tester. His setup is SAS Controller
> > <--> SAS Expander <--> SAS Expander <--> SAS drives
> > <--> SAS Expander <--> SAS drives
> > ......
> > When this issue occurred, no SAS drives could be detected.
> > As a workaround, I've already added those codes removed by the commit a1b6fb947f923,
> and at least we could detect the SAS drives.
>
> ok, good to know, but it would be good to confirm that just re-adding the code to set
> phy_state is enough.
>
> >
> > Since our SAS expanders are self-configured, we don't need to explicitly configure the
> per-PHY routing tables.
> > So, I am not sure there is any other issue in this workaround as some per-PHY routing
> tables are not configured.
> >
> >>>> I think the root cause of this issue is the order of function calls
> >>>> to
> >>>> sas_dev_present_in_domain() and sas_ex_join_wide_port().
> >>>> My concern here is whether or not we still need to configure
> >>>> routing on the parent SAS expander before calling sas_ex_join_wide_port().
> >>>> This part of logic is not present previously and I don't have environment to test this.
> >>>>
> >>>> Back to your question, I believe we do need to join a wideport to downstream expander.
> >>>> Changing the phy_state to PHY_STATE_DISCOVERED will skip scanning
> >>>> those PHYs,
> >> I would have thought that re-adding the code removed in a1b6fb947f923
> >> to set PHY_STATE_DISCOVERED would only affect the first phy scanned
> >> in that wideport. Other phys scanned would have it set through calls
> >> to
> >> sas_ex_join_wide_port()
> >>
> > I don't catch your point.
> > In my understanding, it would affect the rest PHYs in that wide port, not the first one.
> > The first PHY has been scanned/discovered and added to a port (at that time, it is just a
> narrow port yet).
>
> Agreed
>
> > Call to sas_ex_join_wide_port() makes the rest PHYs associated with that existing port
> (making it become wideport) and set up sysfs between the PHY and port. > Set
> PHY_STATE_DISCOVERED would make the rest PHYs not being scanned/discovered again
> (as this wide port is already scanned).
>
> If you can just confirm that re-adding the code to set phy_state = DISCOVERED is good
> enough to see the SAS disks again, then this can be further discussed.
>

OK. I will work on that and keep you updated.

Eric Li