From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH v3 04/11] blk-mq: Introduce blk_mq_quiesce_queue() Date: Wed, 19 Oct 2016 15:23:21 +0200 Message-ID: <20161019132321.GD6323@lst.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from verein.lst.de ([213.95.11.211]:41958 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S942710AbcJSOle (ORCPT ); Wed, 19 Oct 2016 10:41:34 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Bart Van Assche Cc: Jens Axboe , Christoph Hellwig , James Bottomley , "Martin K. Petersen" , Mike Snitzer , Doug Ledford , Keith Busch , Ming Lin , Laurence Oberman , "linux-block@vger.kernel.org" , "linux-scsi@vger.kernel.org" , "linux-rdma@vger.kernel.org" , "linux-nvme@lists.infradead.org" > +/** > + * blk_mq_quiesce_queue() - wait until all ongoing queue_rq calls have finished > + * > + * Note: this function does not prevent that the struct request end_io() > + * callback function is invoked. Additionally, it is not prevented that > + * new queue_rq() calls occur unless the queue has been stopped first. > + */ > +void blk_mq_quiesce_queue(struct request_queue *q) If this is intended to be a kerneldoc comment you need to document the 'q' parameter. If not you should drop the magic "/**" marker. > +static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx) > +{ > + int srcu_idx; > + > + WARN_ON(!cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask) && > + cpu_online(hctx->next_cpu)); > + > + if (!(hctx->flags & BLK_MQ_F_BLOCKING)) { > + rcu_read_lock(); > + blk_mq_process_rq_list(hctx); > + rcu_read_unlock(); > + } else { > + srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu); > + blk_mq_process_rq_list(hctx); > + srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx); > + } > +} Can you document these synchronization changes in detail in the changelog? > +static void blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx, > + struct request *rq, blk_qc_t *cookie) > +{ > + if (blk_mq_hctx_stopped(hctx) || > + blk_mq_direct_issue_request(rq, cookie) != 0) > + blk_mq_insert_request(rq, false, true, true); > +} Any reason not to merge this function with blk_mq_direct_issue_request? Otherwise this change looks fine to me.