Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <a84f7c4e-5da1-4357-9001-c379734e500d@oracle.com>
Date:   Thu, 16 Nov 2023 16:54:35 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] scsi: libsas: Fix set zero-address when device-type !=
 NO_DEVICE
To:     yangxingui <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,
        kangfenglong@huawei.com, chenxiang66@hisilicon.com
References: <20231116035241.13730-1-yangxingui@huawei.com>
 <dea5cabb-7d9a-44b0-85e4-878c13233f6a@oracle.com>
 <9cc196d7-0c4f-ef09-53b8-362d5eb599a6@huawei.com>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <9cc196d7-0c4f-ef09-53b8-362d5eb599a6@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V3pyNGg3K2k4ZTV1K0c0aDg3NklkUVd5SHc0SUlPd1dEd0xYQkh4d21rTDdB?=
 =?utf-8?B?ZnNUcnkvT0pxWndMMW1LR2gvRGFySGdhNmhEMDNCbVVZWEJodkJKdE9yMWFz?=
 =?utf-8?B?SWVCemhVZEJsYlJ3U09udlBMcXBLWHVIdFlwSExtWmdROWNYam00T3F4Nm94?=
 =?utf-8?B?bGI4NlRuck5rOWRFZFlwRE5meHhOTW9MdC9Fc0RoOFl6NDI3enhuayt0Q3Z4?=
 =?utf-8?B?UWNyTU1iWVlvOUYyaC9JcERYM0hMWW0rMGdrVlgwbENWWlFlSDVBamdSczRy?=
 =?utf-8?B?RHE3VUJXVUVONjZHdC90ZHp6VWRGeVlEWWFpUFJmaGdRYXM2MVpsa2hOVXNY?=
 =?utf-8?B?SlhZSm10SXFtRnNObmJIOXNIcGlvcHE4OXJoblVBbUxvcFdDSmt0QUpkL1cz?=
 =?utf-8?B?U3ZGWVVWSVplTnZGUk9xSUlSZDJTSVRmVmJJRVVOTnFkMklYU0pJWkJNRTA5?=
 =?utf-8?B?N0RDdGIwbXk4eEJHZjdmM0E3VDB2UVlmQWZWUE5xVDFuMHZaTmNtS3dwSEI2?=
 =?utf-8?B?bWR1NDc1MEhuSkwweEFBSXZ4ZlpHQTQrVWNINWVXUDhnUlUrZ05sckJKQXEv?=
 =?utf-8?B?ODhwdzlPYW5EandNN3F5N040R01Rb1BNOEhjWDdyZEcxZVhaM0srU0NTK3Ri?=
 =?utf-8?B?UDlDbE44QWV6SXBwOWUxRFRobFpzRjE1czROT2MzZWw4cGpqVWVrcFNBQjU1?=
 =?utf-8?B?bUUyRlUxUVV0dFJxcFo2bU9hNEtyUzdkQ3ltc0dJTFlqbkVidFYrTHljUGNz?=
 =?utf-8?B?YldYKzFrSTVKSWl3aDRXZ3kxaU40c1B6cDdmclBnODgrNW51S1YzOWNNZDU1?=
 =?utf-8?B?OWg1NE5CUHAwR0xrWWwxUFp5NkljNUZvZHpqYWdFSFliVjFxRlpoY3VLd2VS?=
 =?utf-8?B?eUpmb2NjTUZMNmxXei9rNk4yS3MzaGNlRnZ3RHdBQ1p5NVNCdEZXQldKd0dQ?=
 =?utf-8?B?dTk5VTVnREYybVlaVERoeGVCVjdyUDd1Y3VkYjNSRkVjc0FiTFVoTXVCVS9q?=
 =?utf-8?B?dzdCNHBkWEZDZVRKdllFT2lJeU93ZkJXd243VVFURlZLcEVpQUxTYU9WUEhZ?=
 =?utf-8?B?ejc5L2xqNThIWkhUYkJieFZsY1RjK1MxODBZRTV4d2VpZzVWZ1FKUGozOEFj?=
 =?utf-8?B?enp3KysxRzlxQ3ltRmMrNUNiMjUwTW9sRzFpdEp2NUZIcm5GZjUyVE53V1VP?=
 =?utf-8?B?MkZzQkp6ZGUrNG0rbGhBZjRwYlZkM0M3eDd1RHlUbVo1cUdLcHFLNi9GZlRv?=
 =?utf-8?B?cWxrYy9Gb2dGNnUxM0tNSURjVFpjUkVSeTBDajE4aXM3MG9EeXZ6L3Vqall5?=
 =?utf-8?B?VnpHKzZiRGQ4ai9qRzVmdVZrTDg4M1hBLzFHWmhOTVNyZ2pDUnMwT1c5Wllj?=
 =?utf-8?B?S0NRMEVnL3ZLSjBxdmw1THdqNGFSaVFFemU0VmZuU1BDWGhLZWFhN3RwcXRS?=
 =?utf-8?B?bU40YlZzYTBxd29EK3V6UDVUWm0wM2t3enZKVW5SQjVNVm9FMWNaWHFJU2lF?=
 =?utf-8?B?VE9PeGc2b1NLdzYwMk1vZTd5RnA0NlZmMHIxUi9Ub01PWjNVYXlxQkVJblN3?=
 =?utf-8?B?MCsxL2NnNFdaaFkvajM2OFVMWjdJMjJDZlU3MXlLN3lUQzdPWWxQcm13UjNK?=
 =?utf-8?B?VEM2UWZSMnlHMHRCOW5TNWFIMG5aUWFRcWNWbWFYcTFYQTFmVWRrV3IzUVds?=
 =?utf-8?B?Wm90clJXNk43UVNpTHQycWV0V3IrRjNwMVVSdmx4STVCNjQ0Qi9ackpweVBS?=
 =?utf-8?B?eGxRbzRGMzg4YkNaWS9tWmQ4QlhRWURvQmNFZkNLbUphMUNHa3lLOHFUWlVo?=
 =?utf-8?B?R2NvQjJySG9mUmp4VW5nMTRsVm9zcnNyTVlEVXkxRXJiUDBJSnRabVRnUnFL?=
 =?utf-8?B?R1YxbkZ3MHIwcTY5RnZNa05EYk9nMm1LVGFWVEN2TWs0QjUzZVZhbDZCblJZ?=
 =?utf-8?B?Y0dCOE13NEc0YVFaWldnWWhPdlA4RzgxWGFSc1AzeVBFMFpua1VPTmRqcW5I?=
 =?utf-8?B?VjZIRmNUVllNakxudVRocTZVRURhL3ZCMDd5TitjVTFUd0JYczV5bjRFR1d3?=
 =?utf-8?B?alRsTzVKOVVEYUZ0amFkcFpDMjMxWTdNWEhBcmdocUwvZ0ZXdy91TTJ4aXNy?=
 =?utf-8?Q?/elfD00u+zMk1WaasqJxgebg/?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 5CR9N25UsXkBOEFc/V6SFVbf6X6pi+lFUkUBWutaZ680EuGOFj9sGgtkKKK0IXhAM2M4PnxLg2ZCCqd0fipfVYDqV559VmE0j8mRZ9vIgAG9zxJ7Jex3un0lzyGrAX99N3VdOG6aInpeFGgX3W004WpkicZoxG0BU5D1zySXkMt183u+R9Af/uxY7mne+kxspB7t8qhGlf//p+S/hu7J5WhT84zfT+s2IWKk3qtcGmUMQG66LlgWlatf8MehmcJvtXu0wKAI+nRDArHnem4RhM4Pq9qjYLGXxZfkcY/APYb7qGlqCxMvg/K3zLqdlJZKZajlMo2Vr1frlFmEsFZlQlEnPyR2cfzt75FxQUxmvoYiwNDoWTQ1Uml9OKH7cBMAkE6UxurzkLABTSjJs/8pFDUs/JWKRS4l0xN7iS2idBKm/Wsyri3zaAastmMun37gR9x+PROzca9CLKyVH/GbG/KPUvh/Frxomxij5ePnIrrv7WjUUSnJ5vZ1L3lFaGem6TXeRVYfivXQHyTk58+M1Y7VIJcpQwoB/Ux3t9dhtCl+FVwSqn3jAhgnlhA4ta/O2dq/0O5atsjHsK24+SPfBKja3RwkcWxNh7NNIw8TO5F8YQkyyQuKMv42e52nxLyo368L5YBBiM80Y82qwashThmIBzcnxdLSJWC5/sepp79grYJtghj7RYVQ1iYiuoLyGk8yQPTETqta+rk1jVntcqh9QczJma6l0DHvUF0kd2cdswFc3NPkNXqRyRChSb8vdQGr7ZdxhgXVfToEt+BW5RXPrAK3KdICKKt2YMhvveXoD0xrnYJIEEz/lzJ9qRmCw+O6XpGW5U+nugf18MDNxqS2NCsXQoUofhXYgCGCLA/hj8/t+GB2rknWzRx/56tV7sPmPJl+ob6RgFUOYWJjFAoxa6yg4aHlSlInOB0glOM=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4fb2317c-93b1-411c-b4aa-08dbe6c4b860
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2023 16:54:38.6537
 (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: 1Umc8235L5RWdC+WZ/3lM/XI0fbczxDplyViuhWYJ2Br+9Z0UIL9OBQ4w3Mw6gdNo6c8ENMTPoQ/mVpvIb3fJQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4855
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-11-16_17,2023-11-16_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxscore=0 bulkscore=0
 phishscore=0 suspectscore=0 mlxlogscore=999 spamscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000
 definitions=main-2311160132
X-Proofpoint-GUID: waPfHWJrtuFUNSNj7vZJH52ECt9t7Ws3
X-Proofpoint-ORIG-GUID: waPfHWJrtuFUNSNj7vZJH52ECt9t7Ws3
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 16 Nov 2023 08:55:21 -0800 (PST)

On 16/11/2023 13:45, yangxingui wrote:
>> I think that patch title can be improved, but I would need to know 
>> more about the problem before suggesting an improvement.
> How about "Fix port add phy failed" ?
>>
>>> Firstly, when ex_phy is added to the parent port, ex_phy->port is not 
>>> set.
>>
>> That seems correct, but why mention this now?
>>
>>> As a result, sas_port_delete_phy() won't be called in
>>> sas_unregister_devs_sas_addr(), and although ex_phy's sas_address is 
>>> zero,
>>> it is not deleted from the parent port's phy_list.
>>
>> I am not sure why you mention this now either. You seem to be 
>> describing how the problem occurs without actually mentioning what the 
>> problem is.
>>ohn
>>>
>>> Secondly, phy->attached_sas_addr will be set to a zero-address when
>>> phy->linkrate < SAS_LINK_RATE_1_5_GBPS and device-type != NO_DEVICE 
>>> during
>>> device registration, such as stp. It will create a new port and all 
>>> other
>>> ex_phys whose addresses are zero will be added to the new port in
>>> sas_ex_get_linkrate(), and it may trigger BUG() as follows:
>>
>> I think that it would be better to first mention this crash, i.e. the 
>> problem, how you recreate it, and then describe how and why it 
>> happens, and then tell us how you will fix it
> How about follows:
> 
> The following processes trigger a BUG(). A new port port-7:7:0 that 
> created by a new zero-address sata device tries to add phy-7:7:19 had 
> the same zero-address, but phy-7:7:19 is already part of another port.

I would like to know how to recreate, which gives a lot more context and 
helps me understand what the problem is.

Thanks,
John