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: <eabf6fff-93da-6d01-0abe-2f29602b0afb@oracle.com>
Date:   Tue, 22 Nov 2022 10:25:46 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH v2 1/2] scsi: increase scsi device's iodone_cnt in
 scsi_timeout()
Content-Language: en-US
To:     Wenchao Hao <haowenchao@huawei.com>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Steffen Maier <maier@linux.ibm.com>, liuzhiqiang26@huawei.com,
        linfeilong@huawei.com
References: <20221021235638.1968832-1-haowenchao@huawei.com>
 <20221021235638.1968832-2-haowenchao@huawei.com>
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <20221021235638.1968832-2-haowenchao@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3hlUnU1UjBEK3JMZFM3OVk0REpSSkF2UzhnSk1VZkdmYmlNZzJjam5WM25Q?=
 =?utf-8?B?VEtaenlOd0RaRGxneDhLODlzVThvOWdsM0lwRExSMTlvT05tSjY5Snl6RjFm?=
 =?utf-8?B?WVRTd2QrcTdHNVVvbCs4ZDN4WkxyWFFTek5PR3hhdlZvTEZnKzQ5QzlxZnZE?=
 =?utf-8?B?aFRBOUM4ZDZCWVFKT3dIWjRyNjAyTldOaWJ3Tm9ZdW5zaFdsMkl6WTBPbEg4?=
 =?utf-8?B?MHhlV21sY1VXVmpYL3J0Rm96UjdEb2VsWGgzM1d4dU85dXNpbGVKQUozcU9C?=
 =?utf-8?B?Rmd1V1JiUUR1WjRZSk1mNlNsWFlNQS9xWHdzakt5ME1YSS95aDFtZ2NJZnF5?=
 =?utf-8?B?SDYyREY2dURacDZwcW9JOTZRL05yUVcxTWhxMnUvQlBGTWJhQjFrcnJpZ2pZ?=
 =?utf-8?B?MHpmMElRZDIxYWZmcW1DNVlZWTBENFU1bHI5bWVmNXdXWU94TFJuVWZLcGxG?=
 =?utf-8?B?Rjc3U1BXT0hMSHRFUVU1ZTB0U1JMbXhUZm1WU1JyQkZ6aVhRV0JseWdpa3FG?=
 =?utf-8?B?T0dZeFp3QVd0SGdDZDVPYmEwTzlQNUxxT2hXOHB6bzVScmNlOTF0SEhVREtF?=
 =?utf-8?B?UHBEVjVrQVFlaVcxZnd1QmUzT1JlL3EvZGw0TC96NVc1akdibDdtRXA2ZWtj?=
 =?utf-8?B?NWdtRVVrTUVvWEw5MEhsMmVwT0tNditxSWF1ZiswNDZwRXhIRVBxeXNMQlVK?=
 =?utf-8?B?V0dBVFlnMlhlN0xHWGNYRFBaa21LR1o1M25HUk5LQ2tTUmowbGQ3aFZSTTF1?=
 =?utf-8?B?UVNPS0dKRTIyZzBCZ1RrUjJSVkJEWWYvSURIcmNHZFd5dzg5dlZibkZqWk1y?=
 =?utf-8?B?Ynk1OUdNa3hqQUJ1Lzl1Skx1UWo0TDVxcU5lK20wWi9rSWN1RmtxNy9rUWhO?=
 =?utf-8?B?d3o2TFRXUU5OZzgyZEtWVkFnYUd0TElrRWlMN1c5WWxNb1NqRHhsUkMvRzM3?=
 =?utf-8?B?S1hOeDVXS1N0TlhIaWRMYTF4Y1B1RHBmV3hIdmJJM29KTkQzWW82MGNyZmli?=
 =?utf-8?B?NmtZSy9sd1BDNUVjZTRMbURyWFdVZ0laamdxZVFjUEMydzFSZ0llREcwQkpJ?=
 =?utf-8?B?NndsL1NmViswb0g2UHRkUUU2L0V6aU9FSWNmcmcxTTZhdWZyakYzN1pGdDF2?=
 =?utf-8?B?akFhV2RjRXVvQWRNZE1XbmduREMrMEpBUHV3QTRaMFVsblBINkdwLys4Y1U3?=
 =?utf-8?B?djk2QmtOSmxhSWQxVEJLQlZwUW84VDFqRlM2S2NVN1VpQmFLTFRLZjAxSVlL?=
 =?utf-8?B?c25mNCtvMjl4c051NlE0QW9pZ1YwbnZRRUl1YnZwKzdkMGpyQWRub1BOVFpM?=
 =?utf-8?B?bTQ5c1Fmbk1XSy9UZC8wNnpqWlFWTk02ZWVFQjlZVTRrM2pGZWVQQUttQVcr?=
 =?utf-8?B?dTFidi9MSkhPNWo2L0oxNzlQVWtIUlJpb2h4cDFhQTBKNkpGNkdRbWMwQ1Z4?=
 =?utf-8?B?TTc3dDc2T254alRNUDhoelhRc0MxVndjUkQyM2RoWTU3Q09BMlRENVZKYVc4?=
 =?utf-8?B?S3ovZlpEMGo4L3BZOUdWQ0pnRm1EankyMU1YQ1p0Yk9YTktOajlYMHQwMHhv?=
 =?utf-8?B?eE0xNlZLRGVzdGxWWEdpU0RmejlEUXp4Z2YzRzlYZEsyaW5Ic2NHOVhlSzlU?=
 =?utf-8?B?NTJBamZ2YVhuTklGamdDUUVXbkRWOEtPQXl5U3RFTFhLcGdFZDBiaERZcktu?=
 =?utf-8?B?SWF2SFlaNmdYbVhoQXJJaHRCbkl0bjEyOVgvYTVKUDBrczdEYkxSTFMvb24x?=
 =?utf-8?B?RTBsRDlKNklDMXNvc2YvWnVqTzhxVS9PUXRxUE96ZytFUzcwUjdlbGtwY1pV?=
 =?utf-8?B?c3RYNGJ6LzJhV1NFTjRiWUpBbyt1RmN1Tm5lZTJUb3NLQllNMFpWaklJSk1T?=
 =?utf-8?B?VlVmMncwMDZBQWRQak5GYlc2MUk3ejQ0bFF5aDZRd0pKbnBrK0FBcFNHc1M0?=
 =?utf-8?B?V0E5TnNUWkZDbUdDU1R0MzZMSmsrd2dMZVdhMG9FNzUyczZQamNXQU1yL1Qy?=
 =?utf-8?B?N2hjam9iVWd5blFOOFBWL296NXNhMXJVZW05cEpheEhNOVQ2TUpHcVVYQWEw?=
 =?utf-8?B?UnRJYkRMOWZwZTJkMlVIaVl2WGpHZW52YmZ1b2tidWFEZVRDdWwwd0RtQ3R0?=
 =?utf-8?B?VDNaNk4xYnRCeEIwVnRYZmpBbkRRSWdaZFhRTXJ0YzNPeE5zWUVPQVhINVJi?=
 =?utf-8?B?THc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?S0dXNk1STVhRV2h4VEZNak5pd3RlN3ZNUlE2YTFPallRUy9GRW56U25TZ2Z3?=
 =?utf-8?B?ZDR2Qm1uc05kdnFsM0xiSWoxcDF0VjVnaXUxMHVxVkc1ME9DVWIveHk3eE9V?=
 =?utf-8?B?N3JqbFFnTWtSblVYeENGaGhYRW5IYllpNUZIbVpmT3VuYXFaQlhnOWdmVmVX?=
 =?utf-8?B?RzRLOERiZGkrQXlHb2lwby82cXZHSTR4aTAreDZGVlF6Y1hFOGhpVS9EdjZw?=
 =?utf-8?B?UlFudjRidisyVldzay82ZDVyY2xRKzVOUVBkbVhZakM1L2U3QnJLTng5VHNW?=
 =?utf-8?B?NVZTM1E2WU9XNTBSSmgvNlNGeHZZSlg2K0ZRNUFmT3UwRE5oMWlRcmlQV2NY?=
 =?utf-8?B?QXhJSzFsRXdZMUI0UHhOWndHa2o5WUU1QTg3dVFTSUxWemVhbzVYL3hHMFh0?=
 =?utf-8?B?MktqMlo4cW5XNTNqd0xZZ3ppdmxsMUFGdG9ManMxdDg5Y1YxaURucTl5aGNz?=
 =?utf-8?B?STNGT1dIK1hWWExSWjhWVE9oODZZdk5uMTJGVmwxU0FYU0l0cWhnMkJ6RWg5?=
 =?utf-8?B?aElZSmN6RjRjM2Z0MTdTbWNSdkZkaTdIWWlHZDVzTENhN2hiV29RUi9EdE9w?=
 =?utf-8?B?RFhoL3FhRDA0SGNYaERndStBN2E2YWg0bDMwWXd0SXhVMXJzdkRLNEJvckpJ?=
 =?utf-8?B?VVhvWFdrTXY3ZHRvV2F2MW5sL2grZ25oeWUrUVc2YUQvN2JCVWwvRlBRMytC?=
 =?utf-8?B?S0Z4dUpWcTh6Y0VRVmFDSXRsRThxRW9aZ2JxY0lzLzh2Rm1rU2ZPRHc3QUtm?=
 =?utf-8?B?ZzdlczhRLzR0QStrRTRGYkhqL1NRNzZpZkJ2dlhlY290anlkMWFUb0hKV2ZM?=
 =?utf-8?B?bHlDek5wL1NnaTM0M0I1a29PMndDOVBXME1LYnZ6QVFHOExyeDRtdkJ2Ulls?=
 =?utf-8?B?S0w5c3V3V0kxQ2Rreld4aTJJZi83aCs0OWYyaXJtcjZ1M245YlN0VWd4Z1Ft?=
 =?utf-8?B?Sy9KS2hvWGhnNzVOWENXUXJESkZKVUkwQ0dvOWtCZGJ6eXZsaE4xb0xTemtt?=
 =?utf-8?B?a3FhVUN2R3J1OHJReDFlWHVwSDgrMzJLTzlYdSsycjUwZnYvMFZDQlgraDNh?=
 =?utf-8?B?dlJaRWdvSi94NktOaU43OTl2OW11YTBCTVdFalE5d3pZdmhuNUlxOTlVeWhS?=
 =?utf-8?B?NkpSZzhpUjkwRTZpZTN3Nk4zMGlOVnpCM0Jac0FFU0RLOE9CNTVlN0ZtNGhn?=
 =?utf-8?B?dExwRldMUU5YUm82WFBTenBaNHZMMWtrSDZYZEZUQkExNlFjZkJ6V2o0TzFW?=
 =?utf-8?B?V0hpaVgzd25kV0RYSkkvVk1FTFRkSyt4aUYwL0F6T0JVb21BZz09?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0af8476a-8fdb-4aaa-569e-08dacca636ab
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2022 16:25:48.2057
 (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: jQMMxT2myPikN87wqxBrLq9g+Pr/5Wz9wj+voogPjFrrfWdusmcveGUokZNSiUoWDMteE8TZGG1Pe69CXVM4Oi2dmvc9jtJcYUI2bFyTX6E=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5627
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-22_10,2022-11-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 phishscore=0
 adultscore=0 suspectscore=0 mlxscore=0 mlxlogscore=999 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211220124
X-Proofpoint-GUID: NCf86gj02e3mGHbIIKeQGGEWDapmyZIx
X-Proofpoint-ORIG-GUID: NCf86gj02e3mGHbIIKeQGGEWDapmyZIx
X-Spam-Status: No, score=-2.8 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 10/21/22 6:56 PM, Wenchao Hao wrote:
> If an scsi command time out and going to be aborted, we should
> increase the iodone_cnt of the related scsi device, or the
> iodone_cnt would be less than iorequest_cnt
> 
> Increase iodone_cnt in scsi_timeout() would not cause double
> accounting issue, briefly analysed as following:
> 
>  - we add the iodone_cnt when BLK_EH_DONE would be returned in
>    scsi_timeout(), so the related scsi command's timeout event
>    would not happened
> 
>  - if the abort succeed and do not retry, the command would be done
>    with scsi_finish_command() which would not increase iodone_cnt;
> 
>  - if the abort succeed and retry the command, it would be requeue,
>    a scsi_dispatch_cmd() would be called and iorequest_cnt would be
>    increased again
> 
>  - if the abort failed, the error handler successfully recover the
>    device, do not retry this command, the command would be done
>    with scsi_finish_command() which would not increase iodone_cnt;
> 
>  - if the abort failed, the error handler successfully recover the
>    device, and retry this command, the iorequest_cnt would be
>    increased again
> 
> Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
> ---
>  drivers/scsi/scsi_error.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 6995c8979230..052b00f57b56 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -356,6 +356,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req)
>  		 */
>  		if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state))
>  			return BLK_EH_RESET_TIMER;

You will need to rebase this patch because the above line is different now
so it doesn't apply.

It looks ok to me though.

Reviewed-by: Mike Christie <michael.christie@oracle.com>

> +		atomic_inc(&scmd->device->iodone_cnt);
>  		if (scsi_abort_command(scmd) != SUCCESS) {
>  			set_host_byte(scmd, DID_TIME_OUT);
>  			scsi_eh_scmd_add(scmd);