Internal Use - Confidential +AD4- -----Original Message----- +AD4- From: John Garry +ADw-john.g.garry+AEA-oracle.com+AD4- +AD4- Sent: Friday, May 3, 2024 4:33 PM +AD4- To: Li, Eric (Honggang) +ADw-Eric.H.Li+AEA-Dell.com+AD4AOw- Jason Yan +ADw-yanaijie+AEA-huawei.com+AD4AOw- +AD4- james.bottomley+AEA-hansenpartnership.com+ADs- Martin K . Petersen +ADw-martin.petersen+AEA-oracle.com+AD4- +AD4- Cc: linux-scsi+AEA-vger.kernel.org +AD4- Subject: Re: Issue in sas+AF8-ex+AF8-discover+AF8-dev() for multiple level of SAS expanders in a domain +AD4- +AD4- +AD4- +AFs-EXTERNAL EMAIL+AF0- +AD4- +AD4- On 03/05/2024 04:15, Li, Eric (Honggang) wrote: +AD4- +AD4- John, +AD4- +AD4- +AD4- +AD4- I agree that the call to sas+AF8-ex+AF8-join+AF8-wide+AF8-port() is not mandatory. In +AD4- +AD4- fact, some logic here is similar to that function. We don't need to do +AD4- +AD4- it again. +AD4- +AD4- But just updating the phy+AF8-state may not be enough. I suppose you still +AD4- +AD4- need to add that PHY into the corresponding wide port by calling +AD4- +AD4- sas+AF8-port+AF8-add+AF8-phy() and update phy-+AD4-port. +AD4- +AD4- Just updating the phy+AF8-state may avoid the port disabled in this issue, +AD4- +AD4- but other missing piece of work may cause other issues. +AD4- +AD4- +AD4- +AD4- If you check the commit in which that call to sas+AF8-ex+AF8-join+AF8-wide+AF8-port() was originally added - +AD4- 19252de - it was added together with the comment +ACI-Due to races, the phy might not get +AD4- added to the wide port, so we add the phy to the wide port here+ACI-. However the code to set +AD4- phy+AF8-state +AD0- PHY+AF8-STATE+AF8-DISCOVERED already existed before that commit. +AD4- +AD4- When all that code was removed in a1b6fb947f923, I am just wondering if we have kept the +AD4- phy+AF8-state +AD0- PHY+AF8-STATE+AF8-DISCOVERED code. +AD4- +AD4- Anyway, would we really join a wideport with the downstream expander? I would just +AD4- assume that we would be creating a new wideport, and a subsequent scanned phy would be +AD4- added to it. +AFs-Eric: +AF0- I don't think the code removed in a1b6fb947f923 is the right way to fix this issue. It just happened avoiding this issue occurring. I think the root cause of this issue is the order of function calls to sas+AF8-dev+AF8-present+AF8-in+AF8-domain() and sas+AF8-ex+AF8-join+AF8-wide+AF8-port(). My concern here is whether or not we still need to configure routing on the parent SAS expander before calling sas+AF8-ex+AF8-join+AF8-wide+AF8-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+AF8-state to PHY+AF8-STATE+AF8-DISCOVERED will skip scanning those PHYs, by not calling the function sas+AF8-ex+AF8-discover+AF8-dev() to subsequential PHYs within this port (so this issue wouldn+IBk-t hit). But those PHYs are not associated with a SAS port. This may trigger other issues (for example, any change count changed on that PHY, or SAS topology in sysfs, etc.) +AD4- +AD4- +AD4- +AD4- Eric Li +AD4- +AD4-
