| Message ID | 1654770559-101375-2-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | blk-mq/libata/scsi: SCSI driver tagging improvements | expand |
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>
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
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>
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 --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.
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(+)