From:   John Garry <john.g.garry@oracle.com>
To:     jejb@linux.ibm.com, martin.petersen@oracle.com
Cc:     linux-scsi@vger.kernel.org, bvanassche@acm.org,
        linux-kernel@vger.kernel.org, dgilbert@interlog.com,
        John Garry <john.g.garry@oracle.com>
Subject: [PATCH RESEND v2 10/11] scsi: scsi_debug: Get command abort feature working again
Date:   Mon, 13 Mar 2023 09:31:13 +0000
Message-Id: <20230313093114.1498305-11-john.g.garry@oracle.com>
In-Reply-To: <20230313093114.1498305-1-john.g.garry@oracle.com>
References: <20230313093114.1498305-1-john.g.garry@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?azplPkTzzmKPDMvRi+frb1rErengZTK57h1+ihjcuwKXb6FIZ1x32zAzwsoV?=
 =?us-ascii?Q?en5aocaz5hPo/o7jIotjgWhoxXZ+Z06QmMj2u4JMMqYK5pbMSzWBhHGgK/of?=
 =?us-ascii?Q?ccWAP/jUm7XFwyq1H2WUJ7Zvc8wF54l0AzA1PrxP3e/7TlbnUMDFL4nCHv2/?=
 =?us-ascii?Q?crvn80535TVymKwkSGwv1vQwFGe0Vg8DPkj3tAbK+rP2zhcf5fW/ulpCNQ4T?=
 =?us-ascii?Q?YwBJ2ENWKQqQHzLPSmqkKXcGqehFPiZTgM5qJqufwPtSsgeAGSeIoLSdr76t?=
 =?us-ascii?Q?65B4sn0gtUVYp0Nuqll16PRRsXHjhW3klBBFUnlAaLLmFFnnn4ggBdRyeZ3g?=
 =?us-ascii?Q?TbvCdtqGGqwQC0pVmXMEeeN9ACPHbxqYr9YEaIzyeWDiGUzKJH2pbAAw3mGw?=
 =?us-ascii?Q?kQAEbclq5ENXGc4gNRvb+8IvFIdsrsZFY5Y6McU7E19WIMnJKzSvY3BObrOo?=
 =?us-ascii?Q?P/zuZPf/8HkFKnoi9OonbrbBcRUciLsZQBx9V+D2DvWfr9VyeasrCMMXghqA?=
 =?us-ascii?Q?Gc1//BVWtDb4KP6Nr2/GZfY3ZttL2mPOtvCk4+4rlzqpQ5EWH50LtvSVz6b1?=
 =?us-ascii?Q?gLY9bFZ9BnLRgCsnS5onDgiucNpO4holSdVo+BTv6wgHGg3gAEcCCmv/YlDq?=
 =?us-ascii?Q?+GBVJAhrZzBKYs3/5Ohw5ctiIPh0q2NcnfbygyO6Z19xi/jXapeAMPO0PCZT?=
 =?us-ascii?Q?hVhDsvRMgQheCLPYM+sDfthHx9mJ3+pLuNMO2drpR+HGDaaBAZL80TwX9BFw?=
 =?us-ascii?Q?kn9yoeHTvn4oqVpetSoCP9fKquwR4kADFPysj2g1eIacrays/gmcIWQCzxan?=
 =?us-ascii?Q?UcrqmY8Dz2N0/5EtyKO5MsB7SR6AA4VIejsSLEIothugs1B9hdRb+eQRSXwP?=
 =?us-ascii?Q?VLwA0IvVaBPbDjAtNSKoKAhkwosrvlkVY/NbRgC9lxI3tHGQU2MP+UnIGpmZ?=
 =?us-ascii?Q?4PQSHR0cw/hdtxYm4tses6rphWNUES5qEe7bbsN8PGZVUXgSTTua33x+GaKy?=
 =?us-ascii?Q?mh0h6KloSbbn4/HgzUiDB5D2l6Mw1cBOA6WDyj79SSgbU9ebOchSyUs4vPU2?=
 =?us-ascii?Q?auZqp4ysUyQr3PWDoQlJkKsRn684lSHCwgiqg5Ik7xRbmQQNRnmIeLtjyX8P?=
 =?us-ascii?Q?UGwc9URlsLDJAMKZ2UjXCvUWxzAt5JwJWQ3eFk5MdBXlEOOnQeAIelVY/v0S?=
 =?us-ascii?Q?THgCzNObxS1dXie5cpnxus3OTUCPVX2l7YQvp6cVjwmZKTkVYBhtaYyxQmE5?=
 =?us-ascii?Q?bXwHEQ9Xi1lkLEcXiy/EKhga0XmlIFbcIoSoHgf2V4YLFDutV/PxLOodGA0T?=
 =?us-ascii?Q?EumH/QGmWpSAYPr1KS2+7xMtnn14CJYk4yAvqc3hxD74Bosb/i3wfisbhVkB?=
 =?us-ascii?Q?ayzS0JAP+ld/E1oH3WGfrV6tjxCESxAashybWTphMXe9h1wj3V5FPNHYrP4b?=
 =?us-ascii?Q?/wax4OoCeuvNKcpDI1O1QiAD/jgBi9f1oN7GYT80x3/d3T8PFJr0zA1SJzKj?=
 =?us-ascii?Q?ptVf46mbQo8LEdir4Qyqyx8D+Uf5AgVuj+OIwmJ9M9NfnPwJ1srRsELu6WVN?=
 =?us-ascii?Q?eSDpO8MAJO++xEgIqSnAP+VX/gRxAmkhrvccfrmwxfy7prV3rtbEhmRQE0Zc?=
 =?us-ascii?Q?Wg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: Cw3/5c0/7HR9Jmb+TPJt7G3ZiyaQOlyA1ntvOVSTZuOXUTZriZMUScPbgPjmYEdbgGnDTH/BN98upD/QV01bWd1nOaSLyu9BnC5lkv9dYUdZWkMCpo5DjtEbdaGcK9H58nua1Kv4AS0foyZJSZZ0M1sEEkcD4O9FQOtL2JGPCHTFNpLpPpx+QW8A2v94ic/QfuKGWM+8P/jJtBivZWhgBNxcA4E+ojMAQxijItD2x2C94idgX2hbZ/j4OKgqxB+z5X+9gB2HustY+KpnBa3lbrDTOytKtMp/QJE7GedO66OR2fye/Ff+eBelTZFvvPWez3yifiVcJp0gCKuYcmSToYk/lOVHSdsXPE/WWAQTLO+wfTNAwcr+6XNY70PigkiUOJgNAB2OmwyRW1L+R3GF9utoPWLdROYtyo5M2OBiD/L2mPG7xZ7UHUoyraTKtwZFXvu9gCVx2KLCYRyaIrkJSfaK26XEPhh5LuSrHPUrFtR6yEHKmIKaTsmaWbAXzf/0EYLZBI9bHcZEOcQIPytsJhe5h489UbC+A13R/mWcfkzmN+CLrJF9GDjBBEYXAdEByeSPCzwIDo6mki44chOTcROHo6xTyXQrywq/PizE3/W7P//MOmokd1c2yoqFiKdnQdMm9gJRqhsb6ON1YZjM4vJ6y28ldAAcTYYwos+H8iYCK0+emsalyCwvUlPqPnFJYIAqZjC8vjB62OlXtTofwR+Z/adnwI2Wn+WFXTKCIewpEVBOEWSoIRw8n8I+++V2zcQF7oAlOsXkwn+nLu3SGJDcdpCoZ1f26YhbFr7f26pCYYo+gSioS5oRPDZHeV89iFSx05ESzEp1B+g+4NeylhlMVegbp01wPhQlBEQy52s=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9eefe6b7-4c33-464b-6690-08db23a5cd81
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Mar 2023 09:32:02.9786
 (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: a3Vp25b33RL0kQjPz+dhHqG9WD0BR93oUS809xSifBMJAh2WPgtFM3f8yLn7VNxO4l/afferICegxBrPArz/Tw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5060
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-03-13_02,2023-03-10_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0 malwarescore=0
 mlxlogscore=999 mlxscore=0 suspectscore=0 bulkscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2303130077
X-Proofpoint-ORIG-GUID: RxDIV8qTdUMJnMIM8kpD99iDWVs4SK9q
X-Proofpoint-GUID: RxDIV8qTdUMJnMIM8kpD99iDWVs4SK9q
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

The command abort feature allows us to test aborting a command which has
timed-out.

The idea is that for specific commands we just don't call scsi_done() and
allow the request to timeout, which ensures SCSI EH kicks-in we try to
abort the command.

Since commit 4a0c6f432d15 ("scsi: scsi_debug: Add new defer type for
mq_poll") this does not seem to work. The issue is that we clear the
sd_dp->aborted flag in schedule_resp() before the completion callback has
run. When the completion callback actually runs, it calls scsi_done() as
normal as sd_dp->aborted unset. This is all very racy.

Fix by not clearing sd_dp->aborted in schedule_resp(). Also move the call
to blk_abort_request() from schedule_resp() to sdebug_q_cmd_complete(),
which makes the code have a more logical sequence.

I also note that this feature only works for commands which are classed
as "SDEG_RES_IMMED_MASK", but only practically triggered with prior RW
commands. So for my experiment I need to run fio to trigger the error on
the "nth" command (see inject_on_this_cmd()), and then run something like
sg_sync to queue a command to actually trigger the abort.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/scsi/scsi_debug.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 449b460e4c1b..1463e54179bf 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4983,7 +4983,8 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
 	spin_unlock_irqrestore(&sqp->qc_lock, iflags);
 	if (unlikely(aborted)) {
 		if (sdebug_verbose)
-			pr_info("bypassing scsi_done() due to aborted cmd\n");
+			pr_info("bypassing scsi_done() due to aborted cmd, kicking-off EH\n");
+		blk_abort_request(scsi_cmd_to_rq(scp));
 		return;
 	}
 	scsi_done(scp); /* callback to mid level */
@@ -5712,8 +5713,13 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 			sd_dp->issuing_cpu = raw_smp_processor_id();
 	} else {	/* jdelay < 0, use work queue */
 		if (unlikely((sdebug_opts & SDEBUG_OPT_CMD_ABORT) &&
-			     atomic_read(&sdeb_inject_pending)))
+			     atomic_read(&sdeb_inject_pending))) {
 			sd_dp->aborted = true;
+			atomic_set(&sdeb_inject_pending, 0);
+			sdev_printk(KERN_INFO, sdp, "abort request tag=%#x\n",
+				    blk_mq_unique_tag_to_tag(get_tag(cmnd)));
+		}
+
 		if (polled) {
 			sd_dp->cmpl_ts = ns_to_ktime(ns_from_boot);
 			spin_lock_irqsave(&sqp->qc_lock, iflags);
@@ -5738,13 +5744,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
 		}
 		if (sdebug_statistics)
 			sd_dp->issuing_cpu = raw_smp_processor_id();
-		if (unlikely(sd_dp->aborted)) {
-			sdev_printk(KERN_INFO, sdp, "abort request tag %d\n",
-				    scsi_cmd_to_rq(cmnd)->tag);
-			blk_abort_request(scsi_cmd_to_rq(cmnd));
-			atomic_set(&sdeb_inject_pending, 0);
-			sd_dp->aborted = false;
-		}
 	}
 
 	return 0;
-- 
2.35.3