Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9b8da72d-f251-9c1b-0727-28254d7007c3@oracle.com>
Date:   Mon, 19 Dec 2022 14:53:32 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH V2] scsi: libsas: Directly kick-off EH when ATA device
 fell off
To:     yangxingui <yangxingui@huawei.com>, jejb@linux.ibm.com,
        martin.petersen@oracle.com, damien.lemoal@opensource.wdc.com,
        linux-ide@vger.kernel.org, hare@suse.com, hch@lst.de
Cc:     linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linuxarm@huawei.com, prime.zeng@hisilicon.com,
        kangfenglong@huawei.com
References: <20221216100327.7386-1-yangxingui@huawei.com>
 <565fcf28-ec53-8d74-00a3-94be8e5b60e4@oracle.com>
 <f15c142c-669d-6bc7-f9b9-c05cc3df1542@huawei.com>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <f15c142c-669d-6bc7-f9b9-c05cc3df1542@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?MDdLWkhNY29xTmJDTUNhTU5Td1FBVkpvZ1ozOTdKSDVMZDhrSW5hK1dsQzJL?=
 =?utf-8?B?NEpyYXpCMjZZdm9WOG96MDl1Nlkzb2ZCNUpYd2l6TjVPa3BMMTZ3RmQ1VE5L?=
 =?utf-8?B?RFVXMkd4QUlaS2doZ2dkanJGL1ZrVS9qZEYvVDExTlRHd3RUeHdSUmFRclQ4?=
 =?utf-8?B?TzZQZ2lPSGNMN3d5U0ZLV3lHVkN1M0JBeFBubEtseVFTNHc0Wk00YVgvem9Y?=
 =?utf-8?B?K3lBNEV2ZHpkbm5aVjR2MllLK292L1MwM3IxTlhwZlZGNGVBeUhXWHNQRmxG?=
 =?utf-8?B?dXlHNS9COVF4enAvVkZzSHB1VjUyWkpSQ1FJMHRyQ2hLUDNiQnJmR1VoTitm?=
 =?utf-8?B?NUNzd0NoaWcrdW9iUjRISWY2RkNGQSs0S1FYUGNTbWZyaU4vd280MmxINUVM?=
 =?utf-8?B?aVcyR2hJbW0yb0NIVzlFNzVpMFRtQW5nS1dPSHJwd0Q0L2ZoUWJQOXlCZGgr?=
 =?utf-8?B?d1UyUUxPbllrWncwTFpvUjhmVjUzNnFJQTBQbTRKYjhVTUloaERrSFZxbS9U?=
 =?utf-8?B?L2VHWEFwZ0U1V0VQdTB2MEVwcjJ5aHF2YnRQb1d0cHNsWEc3UGNkQnpYZTlU?=
 =?utf-8?B?dkc4eC9GTWg2SG9WMkVPWlNUS08wbkZHQzduY3VxM2k4NnpyRUxPZ1lqcXBH?=
 =?utf-8?B?ZUY1cmlKazZGdHJ6aGwyOFpHWFA0R0RaMWZLZlZaSmlOclNvMHQ4RXp2YTJm?=
 =?utf-8?B?OGJRb3ArNWJoUDYraHE5MmhCS1lINzFlcUJCQkowbHBPbEp1RGRkYlE2cS81?=
 =?utf-8?B?bzZpVkozUU1FY1NrZ2hrVVJJL2FOK0dMRmpkQ0NUQ0tYM3NLZEd5NUtlYkF1?=
 =?utf-8?B?VXRQQWVsUnpLSW93TlhZZE1ZNkExRGFhaTgyc3RJMUwyL1dmUkczemVYWVpG?=
 =?utf-8?B?cTN3WnJqa0ViODQ3a1JwKzlmNnpKQXVsNzV3SHgzblAraGZrLzBtTEtHM2lZ?=
 =?utf-8?B?bUM3OGpTaXB2NUlNRmZrNmFraWxVbnVLZTUyOFBFL2RhREdLT3ZrY09EVW5C?=
 =?utf-8?B?c1ZmVWUzaEUwaEFpdGplVnhmZjllL213aXJHVUhNWVFFNXV5RWo1aEs1T1do?=
 =?utf-8?B?dlRROUNveVdIT2RlSFBEWXFXZlRyMU5nZVZZdDVqaitxdDAvN0FzNUI5N2hD?=
 =?utf-8?B?YmxsMUhsbTE3UkFSZUhDNjBZQmhQeWtZbmdTVVlkSWxqK2pBek5GcWoyOG5G?=
 =?utf-8?B?MmFiTlZ4L1hqMS9QV1VaWmgwTC8xNllYcUQyMzlFaUNsdlgyNnkxVXVSOENC?=
 =?utf-8?B?RTJLd29XbDk1MERwMi9BZ1pxUE5JZDNqUjhwOUFLcVBCeURwbjVXREFZRStq?=
 =?utf-8?B?V3ZlWjBOMFRqOXNCQVRYcE54eElVcGFZMmxjRWhMKzUxU3ZNV3ZVeE42bElp?=
 =?utf-8?B?L1dGS0xHRjdhU2s0OXRJekdBZy9yQmxldUp1OWRUVlVOWUkyQ3FFVUZaOWkv?=
 =?utf-8?B?MTBKMVFQUXc1MTh5V001bDljbTlhZ3lJdzJZaHYzU3pzVjY5N0hVeUlvY1JJ?=
 =?utf-8?B?L2JQbUJIMTJQdHk4emtvekxJbW1yS3ZxMm5ZNldVaXN1ZVpUTTFhTEhLYTM5?=
 =?utf-8?B?RjJFR2lWbWFyNnNwZnlrYVJUQnViT0NGdlZQQjZBZ1Bmc05rUnZvempOMy90?=
 =?utf-8?B?TmVKRFMyT2c3K0U2NW8wdHNHS2haY2xHd3ZudWJ0TVJ0ZlZMbm95VDRJenZj?=
 =?utf-8?B?RC8wK1BGaTdCN3FnRVpzdndSVVVOL2xVZ2ZiVDNBY0FPWll1SnYrNFoyUEtY?=
 =?utf-8?B?dUFnVmJrT0ZGVlVWNDVMSU9uM0hDVGh5aWlISFl4Q3VkTlRkZmtKWGJ1N0Ri?=
 =?utf-8?B?VmJHQjBvR3hMclpyS1kxdzRvK29Cam9QaWIwS0txZm5vMzZLdzdYb0tidXE3?=
 =?utf-8?B?OXJOa2I2T2h3d3lhTmp2cnB3UmM0MWl5bVZFZC9yTERaaFFYQnhYWnVQcWFP?=
 =?utf-8?B?OE1RWHk3cU1rQ0xzRjkxQ0kySXAzOFhXRnJsR3g3UDJkQWJCcXlPUERHa01w?=
 =?utf-8?B?R2hMM012cEpNeGI2UXB5R3NsWWNXdktvZ0xSU2lHeXU1V3EwcUxoaTJUSGNr?=
 =?utf-8?B?VzJnQTVkaG4rdE5mVE5temJsRUxrVjZXdWlaMTJxeUdFdWo1dFBBY1ZYblJR?=
 =?utf-8?Q?BHDMGmWMEuhXxeEC5MRb+ZyA/?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e85d4b5-4ecc-4593-d02b-08dae1d0cf5d
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2022 14:53:37.7011
 (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: dq0AwHNJNVa9M7GXkmVnYOxeow5dXOPxrLw32NG+ahqRUVlLLRH47bTSc9y6rJnA2c8l5iuJ1wowwd+nSVLWmA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB6195
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-12-19_01,2022-12-15_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 bulkscore=0 mlxscore=0
 adultscore=0 phishscore=0 suspectscore=0 spamscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2212190133
X-Proofpoint-ORIG-GUID: iUFSTV0rQJaFtDAeEf3NmFhKxytZN-bl
X-Proofpoint-GUID: iUFSTV0rQJaFtDAeEf3NmFhKxytZN-bl
X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 19/12/2022 12:59, yangxingui wrote:
>> Firstly, I think that there is a bug in sas_ata_device_link_abort() -> 
>> ata_link_abort() code in that the host lock in not grabbed, as the 
>> comment in ata_port_abort() mentions. Having said that, libsas had 
>> already some dodgy host locking usage - specifically dropping the lock 
>> for the queuing path (that's something else to be fixed up ... I think 
>> that is due to queue command CB calling task_done() in some cases), 
>> but I still think that sas_ata_device_link_abort() should be fixed (to 
>> grab the host lock).
> ok, I agree with you very much for this, I had doubts about whether we 
> needed to grab lock before.

ok, I hope that you can fix this up separately.

>>
>> Secondly, this just seems like a half solution to the age-old problem 
>> - that is, EH eventually kicking in only after 30 seconds when a disk 
>> is removed with active IO. I say half solution as SAS disks still have 
>> this issue for libsas. Can we instead push to try to solve both of 
>> them now?
> 
> Jason said you must have such an opinion "a half solution". As libsas 
> does not have any interface to mark all outstanding commands as failed 
> for SAS disk currently and SAS disk support I/O resumable transmission 
> after intermittent disconnections

I don't know what you mean by "resumable transmission after intermittent 
disconnections".

> , so I want to optimize sata disk first.
> If we want to achieve a complete solution, perhaps we need to define 
> such an interface in libsas and implement it by lldd. My current idea is 
> to call sas_abort_task() for all outstanding commands in lldd. I wonder 
> if you approve of this?

Are you sure you mean sas_abort_task()? That is for the LLDD to issue an 
abort TMF. I assume that you mean sas_task_abort(). If so, I am not too 
keen on the idea of libsas calling into the LLDD to inform of such an 
event. Note that maybe a tagset iter function could be used by libsas to 
abort each active IO, but I don't like libsas messing with such a thing; 
in addition, there may be some conflict between libsas aborting the IO 
and the IO completing with error in the LLDD.

Please note that I need to refresh my memory on this whole EH topic...

Thanks,
John