Received-SPF: pass (google.com: domain of linux-kernel+bounces-76526-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <d765d2c5-3451-4519-a5e1-9e8f28dcd6b3@oracle.com>
Date: Thu, 22 Feb 2024 12:41:15 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] scsi: libsas: Fix disk not being scanned in after being
 removed
To: Xingui Yang <yangxingui@huawei.com>, yanaijie@huawei.com,
        jejb@linux.ibm.com, martin.petersen@oracle.com,
        damien.lemoal@opensource.wdc.com
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linuxarm@huawei.com, prime.zeng@hisilicon.com,
        chenxiang66@hisilicon.com, kangfenglong@huawei.com
References: <20240221073159.29408-1-yangxingui@huawei.com>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240221073159.29408-1-yangxingui@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?WjFwQ05jKzRmRmRKNmVMa3hrVVBhWXp1K05pYkc3WlozMjFiR2xWV1lJTFBa?=
 =?utf-8?B?eFRHYlZWanJ2cGlWQjVwdExiNUg4TVZRQlBBUDR3SW1vT3QvU2xVRlYzUVdl?=
 =?utf-8?B?S3BoOS9Ka1VGY2JCVWdQUDNyOGNpdzQyZTdJcGxPRXlHTS80MUx6eGpRRXJ1?=
 =?utf-8?B?bXgzazI3TnEvZ01xZFlpNStqRGNRYUZsYjlTcEoreXE5YXd6K29qNHJoOU9t?=
 =?utf-8?B?SDN0U0U1S2hCRnRyNzJJWVNPT2lLaTlRRXJ0dDdUcjdxZ2Z0OFI5UDlxQUJh?=
 =?utf-8?B?ejBMUkYwemlLejZyNWtZOFp0TTEzYytuNnkzUDMvOHMxZVU4STg0VWJyQWVh?=
 =?utf-8?B?U3VYSm5PdUlyL3VqU0lPTlRta3dKVkcrTkJNU0ZpR3FwVno0ekxEa3FOcU9s?=
 =?utf-8?B?YVdoSUV5cE0xRmZYUlNuMmZxcitHQVFyZStkMnNZTXZvd3Arb1dqTXpHRUNa?=
 =?utf-8?B?REtGK212clBNaXR6cmJENXNSWmlSalFIckd4UGhseGdrdGRsMVJ6andlRjhy?=
 =?utf-8?B?RUUvQzRKYldWb2VUcmhCMW1EMS9wZFJHMkZJSUpCdDNzUzFiaUozdysrZUhw?=
 =?utf-8?B?dDB0LzV0YWRUN052RGJ5VW1qRmZRMmlhRWJtVDl2eC9vbkdVWk9LNHBTSXF6?=
 =?utf-8?B?cVdwc0djWldQdENSb04rZEkzbG5JYlg1YWNDc0hGKzNpdUljNE85bnYvQ0hR?=
 =?utf-8?B?L3NIdnRuaGhGNGhqYmtaVjVxSWhKR2hZcHdKNWc2UFlUUjVrVUFYbEtzMits?=
 =?utf-8?B?alNMN3NDdmRSZm5Ncy9UTHdtSkhrSlo2QjYxeExUdnh3NlNyR2FCMHNxWHRv?=
 =?utf-8?B?Q1FJM0VpM253WGtOMkFKaEMvMWxJUWlrY0pSQUFCRnpjVitUbGE5cys2RjJK?=
 =?utf-8?B?aU1iaGF5dFFTREl0ZFVJcURFY1E2QmcvTnlWSFh0QkNIRGYvaWpYQllCREwv?=
 =?utf-8?B?UWtwc2h4TndBVG9zNlExN2ZWOVRJdFhubFJNaVpjaUNLam95bFFwUnNuTVhm?=
 =?utf-8?B?cVJQc3ZVUU93b1VGRkcvNDlMNDZ3azdzMFYrUUx2TGFNMnR3Y29Jcm9SWjBs?=
 =?utf-8?B?TFVSQzg2NTFWMmRNSEdhR3A3bEhLc1FvUDd3dlJrYUxCNFMwbzRjWVdrdnB3?=
 =?utf-8?B?cHkrZVp5NUNsQ1p4VkJGL1NsNkpFQXJQM0FNSjUvcGozNlJXWGhDeGxvaWs2?=
 =?utf-8?B?L1JZajZYR1YveXBFclJEQzhIWkxHalJtTWF3bm9ic0ZnamZveHBUQUppWWlI?=
 =?utf-8?B?SDVGNW9iNGNpeXBDS3VHVnhpU1RjVS84MVFRUE5WSHRIdlZQTWJvUWZtYzFJ?=
 =?utf-8?B?U0ppenVMK0ZRZXhvMkxpVHhPWGRBNExSQWZTKzBFM1FhZDh3ZUxNbTNYNnEz?=
 =?utf-8?B?eVlGcUwrM2tkckVmNS9SUlZVcFZjeno1K1VxOGRzWHB5VG91MS9INHIzNmtq?=
 =?utf-8?B?NWZHclh4WFpHODZtMVVNZWpFN0c0a0FudHZuS2tNMUFoN2VZV2J6SGVBODRG?=
 =?utf-8?B?VlM5TkRUcDdmV1pjTzlEMk1NQ20wRnpjcmhlK3Q2aGI1OExOaUFOYUhOOEpB?=
 =?utf-8?B?dDJFMmt4WUxlQmJ3SWpVMTBGeXZFdVJweVdLNk8xSkFROGxuUEdGc0NWN1Y3?=
 =?utf-8?B?L0gzQmp2VEZkeGxOSkpjaDNXYlVFY0ZCTng5bTVuWXlIMm9qbjdBai8zakNm?=
 =?utf-8?B?ZkpQUG05R1BESFJKRE5reC9VeDRjejZnRHh2bDVjckhYL2RsM2psTjRCeUhq?=
 =?utf-8?B?TUwrLzl5Y1pQbHJXWDlhMk42QmNQbmJ6bGtEaEFyZG40dzQyK2NhQ2s0dzF3?=
 =?utf-8?B?eU1OMCs0MlRqRk8wT2dRRWlyQVBVbGVHazhkZHpLelBDVkdPZ2ZGazhoWUlR?=
 =?utf-8?B?QnM4NE4rbGRUcGp3SU44UGNOQ0JseC9VUks2QmNUcnVvUk5tSGRheCs2TXFG?=
 =?utf-8?B?bTR4NHFQTHdMV1JXMFFkOU94eHdzaEt5aks2U3ZncVdIRUpYUFFzTjNKUUFW?=
 =?utf-8?B?bW80QTVYTytYTkxrcmpiRWNNbWI4Ujk2L0FJSHN3NjhCbk11c0FiV2EvMUY0?=
 =?utf-8?B?Zjk3aGpleFZGQWl2VmFqazUvbExqSndJOCtiWXJUUnBWQTIyZjhXK1JqTm1T?=
 =?utf-8?Q?ZoUnS0zgS0FBh68IpHSHqi3V2?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	iq35pUpsk0fu54JfHrpd4mzA504/yd4JAuboj5jMHonPlTag3f8HzYx7HyQNVcOsV0zn7u/yb6H/Ei4/g8C0WIUPehuTS1K+xR3u8o7pk6zOYdbqK4V3nexIpKZ8r/Y/A8BSRLFaRsaljC08o453fYKN8q/x4Z6DVvLlTbp2c5fi0026kscPPGdStkszxd0/LfPuk6sLC9pZuibIqcW4V5t3ZG9S38bv6rtalkdpSWL0YwSg5zIbuIvTg4MxfraC+BKL4sJ4nC3CEiKiAEaRsi9z1urX1iesX+Zx/HIrB+F3vdPQi7dZKmknzp2hVrKRkFUs7mEDp+gEVlR/ef2Wp8sNYG6O+DgGutoTqVsiMs8SbHtFVj6jSBLAuSnAuz9CAK9joYOzuJwoPCDxranUfKlp4gcf+HhVUXjykUzNpVNBoQsv0Efs1QCZFe65PSL3bSOjMzACQARdqSlD6GOnrcuVQEg7QaYMM8jcWJNJ7KU+PPYBuWL1ISq7IaXOZdPMCEJwCu6Br64vFiLVZgPg6C0MONQKUb9rB1KwQYTSrKONlJB3No0L2zIrczRQjLrVCBWCKYyXslKwO2At56qipMDmGRA3gXZjMeBxI19w5hQ=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d08f0169-ac98-4e76-69dd-08dc33a39181
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Feb 2024 12:41:19.5739
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: KRKSxLYJd3rQ39XjMZLelrL7c5S2CCPqY4OrnoL0fOQkEtawJl7AkHC/O2kbgO3Ykr2ZBQEfFfhUrdsBuXwtZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB7344
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-22_09,2024-02-22_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 adultscore=0
 mlxlogscore=999 bulkscore=0 malwarescore=0 phishscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402220101
X-Proofpoint-ORIG-GUID: -OdW4M-JGrMw5vIY_J4GceUX2PoLRhe-
X-Proofpoint-GUID: -OdW4M-JGrMw5vIY_J4GceUX2PoLRhe-

On 21/02/2024 07:31, Xingui Yang wrote:
> As of commit d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to
> update PHY info"), do discovery will send a new SMP_DISCOVER and update
> phy->phy_change_count. We found that if the disk is reconnected and phy
> change_count changes at this time, the disk scanning process will not be
> triggered.
> 
> So update the PHY info with the last query results.
> 
> Fixes: d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to update PHY info")
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
>   drivers/scsi/libsas/sas_expander.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index a2204674b680..9563f5589948 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -1681,6 +1681,10 @@ int sas_get_phy_attached_dev(struct domain_device *dev, int phy_id,
>   		if (*type == 0)
>   			memset(sas_addr, 0, SAS_ADDR_SIZE);
>   	}
> +
> +	if ((SAS_ADDR(sas_addr) == 0) || (res == -ECOMM))
> +		sas_set_ex_phy(dev, phy_id, disc_resp);
> +
>   	kfree(disc_resp);
>   	return res;
>   }
> @@ -1972,11 +1976,6 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
>   	if ((SAS_ADDR(sas_addr) == 0) || (res == -ECOMM)) {
>   		phy->phy_state = PHY_EMPTY;
>   		sas_unregister_devs_sas_addr(dev, phy_id, last);
> -		/*
> -		 * Even though the PHY is empty, for convenience we discover
> -		 * the PHY to update the PHY info, like negotiated linkrate.
> -		 */
> -		sas_ex_phy_discover(dev, phy_id);

It would be nice to be able to call sas_set_ex_phy() here (instead of 
sas_get_phy_attached_dev()), but I assume that you can't do that as the 
disc_resp memory is not available.

If we were to manually set the PHY info here instead, how would that look?

Thanks,
John


>   		return res;
>   	} else if (SAS_ADDR(sas_addr) == SAS_ADDR(phy->attached_sas_addr) &&
>   		   dev_type_flutter(type, phy->attached_dev_type)) {