On Wed, Aug 26, 2020 at 08:35:42PM -0700, Bart Van Assche wrote: > On 2020-08-25 18:51, Alan Stern wrote: > > Ah, perfect. So in blk_queue_enter(), pm should be defined in terms of > > RQF_PM rather than BLK_MQ_REQ_PREEMPT. > > > > The difficulty is that the flags argument is the wrong type; RQF_PM is > > defined as req_flags_t, not blk_mq_req_flags_t. It is associated with a > > particular request after the request has been created, so after > > blk_queue_enter() has been called. > > > > How can we solve this? > > The current code looks a bit weird because my focus when modifying the PM > code has been on not breaking any existing code. > > scsi_device_quiesce() relies on blk_queue_enter() processing all PREEMPT > requests. A difficulty is that scsi_device_quiesce() is used for two > separate purposes: > * Runtime power management. > * SCSI domain validation. See e.g. https://lwn.net/Articles/75917/. > > I think that modifying blk_queue_enter() such that it only accepts PM > requests will require to split scsi_device_quiesce() into two functions: > one function that is used by the runtime power management code and another > function that is used by the SCSI domain validation code. This may require > to introduce new SCSI device states. If new SCSI device states are > introduced, that should be done without modifying the state that is > reported to user space. See also sdev_states[] and show_state_field in > scsi_sysfs.c. It may not need to be that complicated. what about something like this? Alan Stern Index: usb-devel/block/blk-core.c =================================================================== --- usb-devel.orig/block/blk-core.c +++ usb-devel/block/blk-core.c @@ -420,11 +420,11 @@ EXPORT_SYMBOL(blk_cleanup_queue); /** * blk_queue_enter() - try to increase q->q_usage_counter * @q: request queue pointer - * @flags: BLK_MQ_REQ_NOWAIT and/or BLK_MQ_REQ_PREEMPT + * @flags: BLK_MQ_REQ_NOWAIT and/or BLK_MQ_REQ_PM */ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags) { - const bool pm = flags & BLK_MQ_REQ_PREEMPT; + const bool pm = flags & BLK_MQ_REQ_PM; while (true) { bool success = false; @@ -626,7 +626,8 @@ struct request *blk_get_request(struct r struct request *req; WARN_ON_ONCE(op & REQ_NOWAIT); - WARN_ON_ONCE(flags & ~(BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_PREEMPT)); + WARN_ON_ONCE(flags & ~(BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_PREEMPT | + BLK_MQ_REQ_PM)); req = blk_mq_alloc_request(q, op, flags); if (!IS_ERR(req) && q->mq_ops->initialize_rq_fn) Index: usb-devel/drivers/scsi/scsi_lib.c =================================================================== --- usb-devel.orig/drivers/scsi/scsi_lib.c +++ usb-devel/drivers/scsi/scsi_lib.c @@ -245,11 +245,15 @@ int __scsi_execute(struct scsi_device *s { struct request *req; struct scsi_request *rq; + blk_mq_req_flags_t mq_req_flags; int ret = DRIVER_ERROR << 24; + mq_req_flags = BLK_MQ_REQ_PREEMPT; + if (rq_flags & RQF_PM) + mq_req_flags |= BLK_MQ_REQ_PM; req = blk_get_request(sdev->request_queue, data_direction == DMA_TO_DEVICE ? - REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); + REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, mq_req_flags); if (IS_ERR(req)) return ret; rq = scsi_req(req); Index: usb-devel/include/linux/blk-mq.h =================================================================== --- usb-devel.orig/include/linux/blk-mq.h +++ usb-devel/include/linux/blk-mq.h @@ -435,6 +435,8 @@ enum { BLK_MQ_REQ_RESERVED = (__force blk_mq_req_flags_t)(1 << 1), /* set RQF_PREEMPT */ BLK_MQ_REQ_PREEMPT = (__force blk_mq_req_flags_t)(1 << 3), + /* used for power management */ + BLK_MQ_REQ_PM = (__force blk_mq_req_flags_t)(1 << 4), }; struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op,
