Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <9fa0cf22-38fd-9390-8f3d-f70e283da302@huawei.com>
Date:   Fri, 17 Nov 2023 17:05:16 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v3] scsi: libsas: Fix set zero-address when device-type !=
 NO_DEVICE
Content-Language: en-CA
To:     John Garry <john.g.garry@oracle.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>
 <a84f7c4e-5da1-4357-9001-c379734e500d@oracle.com>
From:   yangxingui <yangxingui@huawei.com>
In-Reply-To: <a84f7c4e-5da1-4357-9001-c379734e500d@oracle.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi John,

On 2023/11/17 0:54, John Garry wrote:
> 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.
I have update a new version based on your suggestion.

Thanks,
Xingui
.