diff mbox series

[RFC,v2,01/18] blk-mq: Add a flag for reserved requests

Message ID 1654770559-101375-2-git-send-email-john.garry@huawei.com
State Superseded
Headers show
Series blk-mq/libata/scsi: SCSI driver tagging improvements | expand

Commit Message

John Garry June 9, 2022, 10:29 a.m. UTC
Add a flag for reserved requests so that drivers may know this for any
special handling.

The 'reserved' argument in blk_mq_ops.timeout callback could now be
replaced by using this flag.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 block/blk-mq.c         | 6 ++++++
 include/linux/blk-mq.h | 6 ++++++
 2 files changed, 12 insertions(+)

Comments

Christoph Hellwig June 14, 2022, 6:43 a.m. UTC | #1
On Thu, Jun 09, 2022 at 06:29:02PM +0800, John Garry wrote:
> Add a flag for reserved requests so that drivers may know this for any
> special handling.
> 
> The 'reserved' argument in blk_mq_ops.timeout callback could now be
> replaced by using this flag.

And we should probably do that ASAP, independent of the rest of this
series.  Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
John Garry June 14, 2022, 9:29 a.m. UTC | #2
On 14/06/2022 07:43, Christoph Hellwig wrote:
> On Thu, Jun 09, 2022 at 06:29:02PM +0800, John Garry wrote:
>> Add a flag for reserved requests so that drivers may know this for any
>> special handling.
>>
>> The 'reserved' argument in blk_mq_ops.timeout callback could now be
>> replaced by using this flag.
> And we should probably do that ASAP, independent of the rest of this
> series. 

We only have 2x users of the 'reserved' arg for 11x implementations of 
blk_mq_ops.timeout:
- mtip32xx.c
- scsi_lib.c

scsi_lib.c is dubious as currently scsi does use reserved commands. So 
we really only have 1x.

I'd be happy to take any spin-off series to make this one more 
manageable, but is just removing an arg a strong enough reason for that? 
That reserved arg is passed around a lot in the blk-mq iter functions, 
so probably yes.

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig<hch@lst.de>

Thanks
Bart Van Assche June 14, 2022, 6 p.m. UTC | #3
On 6/9/22 03:29, John Garry wrote:
> Add a flag for reserved requests so that drivers may know this for any
> special handling.
> 
> The 'reserved' argument in blk_mq_ops.timeout callback could now be
> replaced by using this flag.

Why not to combine that change into this patch?

Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
John Garry June 14, 2022, 6:30 p.m. UTC | #4
On 14/06/2022 19:00, Bart Van Assche wrote:
> On 6/9/22 03:29, John Garry wrote:
>> Add a flag for reserved requests so that drivers may know this for any
>> special handling.
>>
>> The 'reserved' argument in blk_mq_ops.timeout callback could now be
>> replaced by using this flag.
> 
> Why not to combine that change into this patch?
> 

If we remove the 'reserved' argument in blk_mq_ops.timeout callback then 
we can also remove the 'reserved' member of busy_tag_iter_fn. I gave 
that all a try and the diffstat looks like this:

  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   | 11 +++++------
  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/scsi/aacraid/comminit.c     |  2 +-
  drivers/scsi/aacraid/linit.c        |  2 +-
  drivers/scsi/hosts.c                | 14 ++++++--------
  drivers/scsi/mpi3mr/mpi3mr_os.c     | 15 ++++-----------
  drivers/scsi/scsi_lib.c             | 12 ++----------
  include/linux/blk-mq.h              | 10 ++++++++--
  include/scsi/scsi_host.h            |  2 +-
  24 files changed, 62 insertions(+), 81 deletions(-)

It would seem sensible to send that all separately and break it down a 
bit - this series is already almost unmanageable.

> Anyway:
> 
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> .

Thanks
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e9bf950983c7..23f2eafb09ca 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -474,6 +474,9 @@  static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 	if (!(data->rq_flags & RQF_ELV))
 		blk_mq_tag_busy(data->hctx);
 
+	if (data->flags & BLK_MQ_REQ_RESERVED)
+		data->rq_flags |= RQF_RESV;
+
 	/*
 	 * Try batched alloc if we want more than 1 tag.
 	 */
@@ -586,6 +589,9 @@  struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
 	else
 		data.rq_flags |= RQF_ELV;
 
+	if (flags & BLK_MQ_REQ_RESERVED)
+		data.rq_flags |= RQF_RESV;
+
 	ret = -EWOULDBLOCK;
 	tag = blk_mq_get_tag(&data);
 	if (tag == BLK_MQ_NO_TAG)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index e2d9daf7e8dd..6d81fe10e850 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -57,6 +57,7 @@  typedef __u32 __bitwise req_flags_t;
 #define RQF_TIMED_OUT		((__force req_flags_t)(1 << 21))
 /* queue has elevator attached */
 #define RQF_ELV			((__force req_flags_t)(1 << 22))
+#define RQF_RESV			((__force req_flags_t)(1 << 23))
 
 /* flags that prevent us from merging requests: */
 #define RQF_NOMERGE_FLAGS \
@@ -823,6 +824,11 @@  static inline bool blk_mq_need_time_stamp(struct request *rq)
 	return (rq->rq_flags & (RQF_IO_STAT | RQF_STATS | RQF_ELV));
 }
 
+static inline bool blk_mq_is_reserved_rq(struct request *rq)
+{
+	return rq->rq_flags & RQF_RESV;
+}
+
 /*
  * Batched completions only work when there is no I/O error and no special
  * ->end_io handler.