2022-07-06 12:30:02

by John Garry

[permalink] [raw]
Subject: [PATCH v3 0/6] blk-mq: Add a flag for reserved requests series

Hi Jens,

Can you please consider this series? Thanks

---

In [0] I included "blk-mq: Add a flag for reserved requests" to identify
if a request is 'reserved' for special handling. Doing this is easier than
passing a 'reserved' arg to the blk_mq_ops callbacks. Indeed, only 1x
timeout implementation or blk-mq iter function actually uses the
'reserved' arg (or 3x if you count SCSI core and FNIC SCSI driver). So
this series drops the 'reserved' arg for these timeout and iter functions.
Christoph suggested that I try to upstream now.

Differences to v2:
- Rebase
- Apply more RB tags (thanks!)

Differences to v1:
- Use "scsi_timeout" as name for SCSI timeout function and update docs
- Add RB tags (thanks!)
- Split out patch to drop local variables for 'reserved', as requested by
Bart

Based on following:
e55cf7981405 (block/for-5.20/block) blk-cgroup: factor out blkcg_free_all_cpd()

[0] https://lore.kernel.org/linux-scsi/[email protected]/T/#m22aa9f89e55835edc2e650d43f7e3219a3a1a324

John Garry (6):
scsi: core: Remove reserved request time-out handling
blk-mq: Add a flag for reserved requests
blk-mq: Drop blk_mq_ops.timeout 'reserved' arg
scsi: fnic: Drop reserved request handling
blk-mq: Drop 'reserved' arg of busy_tag_iter_fn
blk-mq: Drop local variable for reserved tag

Documentation/scsi/scsi_eh.rst | 3 +--
Documentation/scsi/scsi_mid_low_api.rst | 2 +-
block/blk-mq-debugfs.c | 2 +-
block/blk-mq-tag.c | 13 +++++--------
block/blk-mq.c | 22 +++++++++++++---------
block/bsg-lib.c | 2 +-
drivers/block/mtip32xx/mtip32xx.c | 9 ++++-----
drivers/block/nbd.c | 5 ++---
drivers/block/null_blk/main.c | 2 +-
drivers/infiniband/ulp/srp/ib_srp.c | 3 +--
drivers/mmc/core/queue.c | 3 +--
drivers/nvme/host/apple.c | 3 +--
drivers/nvme/host/core.c | 2 +-
drivers/nvme/host/fc.c | 6 ++----
drivers/nvme/host/nvme.h | 2 +-
drivers/nvme/host/pci.c | 2 +-
drivers/nvme/host/rdma.c | 3 +--
drivers/nvme/host/tcp.c | 3 +--
drivers/s390/block/dasd.c | 2 +-
drivers/s390/block/dasd_int.h | 2 +-
drivers/scsi/aacraid/comminit.c | 2 +-
drivers/scsi/aacraid/linit.c | 2 +-
drivers/scsi/fnic/fnic_scsi.c | 14 ++++----------
drivers/scsi/hosts.c | 14 ++++++--------
drivers/scsi/mpi3mr/mpi3mr_os.c | 16 ++++------------
drivers/scsi/scsi_error.c | 6 +++---
drivers/scsi/scsi_lib.c | 8 --------
drivers/scsi/scsi_priv.h | 2 +-
include/linux/blk-mq.h | 10 ++++++++--
include/scsi/scsi_host.h | 2 +-
30 files changed, 70 insertions(+), 97 deletions(-)

--
2.35.3


2022-07-06 12:34:13

by John Garry

[permalink] [raw]
Subject: [PATCH v3 1/6] scsi: core: Remove reserved request time-out handling

The SCSI core code does not currently support reserved commands. As such,
requests which time-out would never be reserved, and scsi_timeout()
'reserved' arg should never be set.

Remove handling for reserved requests, drop the wrapper scsi_timeout()
as it now just calls scsi_times_out() always, and finally rename
scsi_times_out() -> scsi_timeout() to match the blk_mq_ops method name.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]>
---
Documentation/scsi/scsi_eh.rst | 3 +--
Documentation/scsi/scsi_mid_low_api.rst | 2 +-
drivers/scsi/scsi_error.c | 7 ++++---
drivers/scsi/scsi_lib.c | 8 --------
drivers/scsi/scsi_priv.h | 3 ++-
5 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/Documentation/scsi/scsi_eh.rst b/Documentation/scsi/scsi_eh.rst
index 885395dc1f15..bad624fab823 100644
--- a/Documentation/scsi/scsi_eh.rst
+++ b/Documentation/scsi/scsi_eh.rst
@@ -87,8 +87,7 @@ with the command.
1.2.2 Completing a scmd w/ timeout
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-The timeout handler is scsi_times_out(). When a timeout occurs, this
-function
+The timeout handler is scsi_timeout(). When a timeout occurs, this function

1. invokes optional hostt->eh_timed_out() callback. Return value can
be one of
diff --git a/Documentation/scsi/scsi_mid_low_api.rst b/Documentation/scsi/scsi_mid_low_api.rst
index 63ddea2b9640..a8c5bd15a440 100644
--- a/Documentation/scsi/scsi_mid_low_api.rst
+++ b/Documentation/scsi/scsi_mid_low_api.rst
@@ -731,7 +731,7 @@ Details::
* Notes: If 'no_async_abort' is defined this callback
* will be invoked from scsi_eh thread. No other commands
* will then be queued on current host during eh.
- * Otherwise it will be called whenever scsi_times_out()
+ * Otherwise it will be called whenever scsi_timeout()
* is called due to a command timeout.
*
* Optionally defined in: LLD
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 49ef864df581..a8b71b73a5a5 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -139,7 +139,7 @@ static bool scsi_eh_should_retry_cmd(struct scsi_cmnd *cmd)
*
* Note: this function must be called only for a command that has timed out.
* Because the block layer marks a request as complete before it calls
- * scsi_times_out(), a .scsi_done() call from the LLD for a command that has
+ * scsi_timeout(), a .scsi_done() call from the LLD for a command that has
* timed out do not have any effect. Hence it is safe to call
* scsi_finish_command() from this function.
*/
@@ -316,8 +316,9 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
}

/**
- * scsi_times_out - Timeout function for normal scsi commands.
+ * scsi_timeout - Timeout function for normal scsi commands.
* @req: request that is timing out.
+ * @reserved: whether the request is a reserved request.
*
* Notes:
* We do not need to lock this. There is the potential for a race
@@ -325,7 +326,7 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
* normal completion function determines that the timer has already
* fired, then it mustn't do anything.
*/
-enum blk_eh_timer_return scsi_times_out(struct request *req)
+enum blk_eh_timer_return scsi_timeout(struct request *req, bool reserved)
{
struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
enum blk_eh_timer_return rtn = BLK_EH_DONE;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cdf0056582d5..1b3ca5c16c3d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1790,14 +1790,6 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
return ret;
}

-static enum blk_eh_timer_return scsi_timeout(struct request *req,
- bool reserved)
-{
- if (reserved)
- return BLK_EH_RESET_TIMER;
- return scsi_times_out(req);
-}
-
static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq,
unsigned int hctx_idx, unsigned int numa_node)
{
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 5c4786310a31..695d0c83ffe0 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -72,7 +72,8 @@ extern void scsi_exit_devinfo(void);

/* scsi_error.c */
extern void scmd_eh_abort_handler(struct work_struct *work);
-extern enum blk_eh_timer_return scsi_times_out(struct request *req);
+extern enum blk_eh_timer_return scsi_timeout(struct request *req,
+ bool reserved);
extern int scsi_error_handler(void *host);
extern enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *cmd);
extern void scsi_eh_wakeup(struct Scsi_Host *shost);
--
2.35.3

2022-07-06 13:11:04

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] blk-mq: Add a flag for reserved requests series

On Wed, 6 Jul 2022 20:03:48 +0800, John Garry wrote:
> Can you please consider this series? Thanks
>

Applied, thanks!

[1/6] scsi: core: Remove reserved request time-out handling
commit: deef1be18e3fc62ddf04fb3e5e8ff6a301693dcc
[2/6] blk-mq: Add a flag for reserved requests
commit: 99e48cd6855e9535488e3c90d65edd46c6e6fc1b
[3/6] blk-mq: Drop blk_mq_ops.timeout 'reserved' arg
commit: 9bdb4833dd399cbff82cc20893f52bdec66a9eca
[4/6] scsi: fnic: Drop reserved request handling
commit: 1263c1929fb8c375494666ec6d1bac838ff02c25
[5/6] blk-mq: Drop 'reserved' arg of busy_tag_iter_fn
commit: 2dd6532e9591f201e7571b30915db807603ab924
[6/6] blk-mq: Drop local variable for reserved tag
commit: 4cf6e6c0106bf6e6d034fa6043b4428ac2f267fc

Best regards,
--
Jens Axboe


2022-07-06 13:15:20

by John Garry

[permalink] [raw]
Subject: [PATCH v3 3/6] blk-mq: Drop blk_mq_ops.timeout 'reserved' arg

With new API blk_mq_is_reserved_rq() we can tell if a request is from
the reserved pool, so stop passing 'reserved' arg. There is actually
only a single user of that arg for all the callback implementations, which
can use blk_mq_is_reserved_rq() instead.

This will also allow us to stop passing the same 'reserved' around the
blk-mq iter functions next.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Acked-by: Ulf Hansson <[email protected]> # For MMC
Reviewed-by: Martin K. Petersen <[email protected]>
---
block/blk-mq.c | 6 +++---
block/bsg-lib.c | 2 +-
drivers/block/mtip32xx/mtip32xx.c | 5 ++---
drivers/block/nbd.c | 3 +--
drivers/block/null_blk/main.c | 2 +-
drivers/mmc/core/queue.c | 3 +--
drivers/nvme/host/apple.c | 3 +--
drivers/nvme/host/fc.c | 3 +--
drivers/nvme/host/pci.c | 2 +-
drivers/nvme/host/rdma.c | 3 +--
drivers/nvme/host/tcp.c | 3 +--
drivers/s390/block/dasd.c | 2 +-
drivers/s390/block/dasd_int.h | 2 +-
drivers/scsi/scsi_error.c | 3 +--
drivers/scsi/scsi_priv.h | 3 +--
include/linux/blk-mq.h | 2 +-
16 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a00e43cc67e5..cedbec36e907 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1427,13 +1427,13 @@ bool blk_mq_queue_inflight(struct request_queue *q)
}
EXPORT_SYMBOL_GPL(blk_mq_queue_inflight);

-static void blk_mq_rq_timed_out(struct request *req, bool reserved)
+static void blk_mq_rq_timed_out(struct request *req)
{
req->rq_flags |= RQF_TIMED_OUT;
if (req->q->mq_ops->timeout) {
enum blk_eh_timer_return ret;

- ret = req->q->mq_ops->timeout(req, reserved);
+ ret = req->q->mq_ops->timeout(req);
if (ret == BLK_EH_DONE)
return;
WARN_ON_ONCE(ret != BLK_EH_RESET_TIMER);
@@ -1482,7 +1482,7 @@ static bool blk_mq_check_expired(struct request *rq, void *priv, bool reserved)
* from blk_mq_check_expired().
*/
if (blk_mq_req_expired(rq, next))
- blk_mq_rq_timed_out(rq, reserved);
+ blk_mq_rq_timed_out(rq);
return true;
}

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index fd4cd5e68282..d6f5dcdce748 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -331,7 +331,7 @@ void bsg_remove_queue(struct request_queue *q)
}
EXPORT_SYMBOL_GPL(bsg_remove_queue);

-static enum blk_eh_timer_return bsg_timeout(struct request *rq, bool reserved)
+static enum blk_eh_timer_return bsg_timeout(struct request *rq)
{
struct bsg_set *bset =
container_of(rq->q->tag_set, struct bsg_set, tag_set);
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index e116c6cf56f5..5073cb407500 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -3357,12 +3357,11 @@ static int mtip_init_cmd(struct blk_mq_tag_set *set, struct request *rq,
return 0;
}

-static enum blk_eh_timer_return mtip_cmd_timeout(struct request *req,
- bool reserved)
+static enum blk_eh_timer_return mtip_cmd_timeout(struct request *req)
{
struct driver_data *dd = req->q->queuedata;

- if (reserved) {
+ if (blk_mq_is_reserved_rq(req)) {
struct mtip_cmd *cmd = blk_mq_rq_to_pdu(req);

cmd->status = BLK_STS_TIMEOUT;
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5c4c9c45c6ac..028f23c965df 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -393,8 +393,7 @@ static u32 req_to_nbd_cmd_type(struct request *req)
}
}

-static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
- bool reserved)
+static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req)
{
struct nbd_cmd *cmd = blk_mq_rq_to_pdu(req);
struct nbd_device *nbd = cmd->nbd;
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index d695ea29efa6..4e03a020ee3c 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1578,7 +1578,7 @@ static int null_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
return nr;
}

-static enum blk_eh_timer_return null_timeout_rq(struct request *rq, bool res)
+static enum blk_eh_timer_return null_timeout_rq(struct request *rq)
{
struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
struct nullb_cmd *cmd = blk_mq_rq_to_pdu(rq);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index f824cfdab75a..fefaa901b50f 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -116,8 +116,7 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req)
}
}

-static enum blk_eh_timer_return mmc_mq_timed_out(struct request *req,
- bool reserved)
+static enum blk_eh_timer_return mmc_mq_timed_out(struct request *req)
{
struct request_queue *q = req->q;
struct mmc_queue *mq = q->queuedata;
diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 2d23b7d41f7e..5c352d5d8ee6 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -862,8 +862,7 @@ static void apple_nvme_disable(struct apple_nvme *anv, bool shutdown)
}
}

-static enum blk_eh_timer_return apple_nvme_timeout(struct request *req,
- bool reserved)
+static enum blk_eh_timer_return apple_nvme_timeout(struct request *req)
{
struct apple_nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct apple_nvme_queue *q = iod->q;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index a96aa831684c..07fd6db5869c 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2565,8 +2565,7 @@ nvme_fc_error_recovery(struct nvme_fc_ctrl *ctrl, char *errmsg)
nvme_reset_ctrl(&ctrl->ctrl);
}

-static enum blk_eh_timer_return
-nvme_fc_timeout(struct request *rq, bool reserved)
+static enum blk_eh_timer_return nvme_fc_timeout(struct request *rq)
{
struct nvme_fc_fcp_op *op = blk_mq_rq_to_pdu(rq);
struct nvme_fc_ctrl *ctrl = op->ctrl;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 247a74aba336..4232192e10dd 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1344,7 +1344,7 @@ static void nvme_warn_reset(struct nvme_dev *dev, u32 csts)
"Try \"nvme_core.default_ps_max_latency_us=0 pcie_aspm=off\" and report a bug\n");
}

-static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
+static enum blk_eh_timer_return nvme_timeout(struct request *req)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct nvme_queue *nvmeq = iod->nvmeq;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 0fb7c8e7ab0b..a6eaf38b9646 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -2013,8 +2013,7 @@ static void nvme_rdma_complete_timed_out(struct request *rq)
nvmf_complete_timed_out_request(rq);
}

-static enum blk_eh_timer_return
-nvme_rdma_timeout(struct request *rq, bool reserved)
+static enum blk_eh_timer_return nvme_rdma_timeout(struct request *rq)
{
struct nvme_rdma_request *req = blk_mq_rq_to_pdu(rq);
struct nvme_rdma_queue *queue = req->queue;
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index b81942fa5f95..ff502172accd 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2321,8 +2321,7 @@ static void nvme_tcp_complete_timed_out(struct request *rq)
nvmf_complete_timed_out_request(rq);
}

-static enum blk_eh_timer_return
-nvme_tcp_timeout(struct request *rq, bool reserved)
+static enum blk_eh_timer_return nvme_tcp_timeout(struct request *rq)
{
struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
struct nvme_ctrl *ctrl = &req->queue->ctrl->ctrl;
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index e8489331f12b..4df8bf6505fc 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -3145,7 +3145,7 @@ static blk_status_t do_dasd_request(struct blk_mq_hw_ctx *hctx,
* BLK_EH_DONE if the request is handled or terminated
* by the driver.
*/
-enum blk_eh_timer_return dasd_times_out(struct request *req, bool reserved)
+enum blk_eh_timer_return dasd_times_out(struct request *req)
{
struct dasd_block *block = req->q->queuedata;
struct dasd_device *device;
diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index 83b918b84b4a..333a399f754e 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -795,7 +795,7 @@ void dasd_free_device(struct dasd_device *);
struct dasd_block *dasd_alloc_block(void);
void dasd_free_block(struct dasd_block *);

-enum blk_eh_timer_return dasd_times_out(struct request *req, bool reserved);
+enum blk_eh_timer_return dasd_times_out(struct request *req);

void dasd_enable_device(struct dasd_device *);
void dasd_set_target_state(struct dasd_device *, int);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index a8b71b73a5a5..266ce414589c 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -318,7 +318,6 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
/**
* scsi_timeout - Timeout function for normal scsi commands.
* @req: request that is timing out.
- * @reserved: whether the request is a reserved request.
*
* Notes:
* We do not need to lock this. There is the potential for a race
@@ -326,7 +325,7 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
* normal completion function determines that the timer has already
* fired, then it mustn't do anything.
*/
-enum blk_eh_timer_return scsi_timeout(struct request *req, bool reserved)
+enum blk_eh_timer_return scsi_timeout(struct request *req)
{
struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
enum blk_eh_timer_return rtn = BLK_EH_DONE;
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 695d0c83ffe0..6eeaa0a7f86d 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -72,8 +72,7 @@ extern void scsi_exit_devinfo(void);

/* scsi_error.c */
extern void scmd_eh_abort_handler(struct work_struct *work);
-extern enum blk_eh_timer_return scsi_timeout(struct request *req,
- bool reserved);
+extern enum blk_eh_timer_return scsi_timeout(struct request *req);
extern int scsi_error_handler(void *host);
extern enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *cmd);
extern void scsi_eh_wakeup(struct Scsi_Host *shost);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 7c62b7fabec7..c84c56d296fe 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -575,7 +575,7 @@ struct blk_mq_ops {
/**
* @timeout: Called on request timeout.
*/
- enum blk_eh_timer_return (*timeout)(struct request *, bool);
+ enum blk_eh_timer_return (*timeout)(struct request *);

/**
* @poll: Called to poll for completion of a specific tag.
--
2.35.3