From mboxrd@z Thu Jan 1 00:00:00 1970 From: hare@suse.de (Hannes Reinecke) Date: Mon, 15 May 2017 08:40:38 +0200 Subject: [PATCH 06/10] nvmet_fc: Reduce work_q count In-Reply-To: <20170513190309.25417-7-jsmart2021@gmail.com> References: <20170513190309.25417-1-jsmart2021@gmail.com> <20170513190309.25417-7-jsmart2021@gmail.com> Message-ID: <5e0fefab-9a20-a409-8ff2-6eefe3ab1649@suse.de> On 05/13/2017 09:03 PM, James Smart wrote: > Instead of a work_q per controller queue, make 1 per cpu and have > controller queues post work elements to the work_q. > > Signed-off-by: James Smart > --- > this is version 3 of the patch: > v2: > converted to use DEFINE_PER_CPU() > reworked do {} while into more readable for loop in > nvmet_fc_do_work_on_cpu() > renamed create/delete_threads to create/delete_workqueues > > v3:recut on nvme-4.12 > > drivers/nvme/target/fc.c | 205 +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 154 insertions(+), 51 deletions(-) > > diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c > index 2006fae61980..c6c3c1ffaf2f 100644 > --- a/drivers/nvme/target/fc.c > +++ b/drivers/nvme/target/fc.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -81,6 +82,7 @@ struct nvmet_fc_fcp_iod { > u32 offset; > enum nvmet_fcp_datadir io_dir; > bool active; > + bool started; > bool abort; > bool aborted; > bool writedataactive; > @@ -88,12 +90,12 @@ struct nvmet_fc_fcp_iod { > > struct nvmet_req req; > struct work_struct work; > - struct work_struct done_work; > > struct nvmet_fc_tgtport *tgtport; > struct nvmet_fc_tgt_queue *queue; > > - struct list_head fcp_list; /* tgtport->fcp_list */ > + struct list_head fcp_list; /* queue->fod_list */ > + struct list_head work_list; /* workcpu->work_list */ > }; > > struct nvmet_fc_tgtport { > @@ -132,7 +134,6 @@ struct nvmet_fc_tgt_queue { > struct nvmet_fc_tgt_assoc *assoc; > struct nvmet_fc_fcp_iod *fod; /* array of fcp_iods */ > struct list_head fod_list; > - struct workqueue_struct *work_q; > struct kref ref; > } __aligned(sizeof(unsigned long long)); > > @@ -145,6 +146,20 @@ struct nvmet_fc_tgt_assoc { > struct kref ref; > }; > > +enum nvmet_fc_workcpu_flags { > + NVMET_FC_CPU_RUNNING = (1 << 0), > + NVMET_FC_CPU_TERMINATING = (1 << 1), > +}; > + > +struct nvmet_fc_work_by_cpu { > + struct workqueue_struct *work_q; > + struct list_head fod_list; > + spinlock_t clock; > + int cpu; > + bool running; > + struct work_struct cpu_work; > +}; > + > > static inline int > nvmet_fc_iodnum(struct nvmet_fc_ls_iod *iodptr) > @@ -213,10 +228,11 @@ static DEFINE_SPINLOCK(nvmet_fc_tgtlock); > static LIST_HEAD(nvmet_fc_target_list); > static DEFINE_IDA(nvmet_fc_tgtport_cnt); > > +static u32 nvmet_fc_cpu_cnt; > +static DEFINE_PER_CPU(struct nvmet_fc_work_by_cpu, nvmet_fc_cpu_workcpu); > +#define nvmet_fc_workcpu(cpu) (&per_cpu(nvmet_fc_cpu_workcpu, cpu)) > > static void nvmet_fc_handle_ls_rqst_work(struct work_struct *work); > -static void nvmet_fc_handle_fcp_rqst_work(struct work_struct *work); > -static void nvmet_fc_fcp_rqst_op_done_work(struct work_struct *work); > static void nvmet_fc_tgt_a_put(struct nvmet_fc_tgt_assoc *assoc); > static int nvmet_fc_tgt_a_get(struct nvmet_fc_tgt_assoc *assoc); > static void nvmet_fc_tgt_q_put(struct nvmet_fc_tgt_queue *queue); > @@ -417,11 +433,10 @@ nvmet_fc_prep_fcp_iodlist(struct nvmet_fc_tgtport *tgtport, > int i; > > for (i = 0; i < queue->sqsize; fod++, i++) { > - INIT_WORK(&fod->work, nvmet_fc_handle_fcp_rqst_work); > - INIT_WORK(&fod->done_work, nvmet_fc_fcp_rqst_op_done_work); > fod->tgtport = tgtport; > fod->queue = queue; > fod->active = false; > + fod->started = false; > fod->abort = false; > fod->aborted = false; > fod->fcpreq = NULL; > @@ -498,6 +513,7 @@ nvmet_fc_free_fcp_iod(struct nvmet_fc_tgt_queue *queue, > spin_lock_irqsave(&queue->qlock, flags); > list_add_tail(&fod->fcp_list, &fod->queue->fod_list); > fod->active = false; > + fod->started = false; > fod->abort = false; > fod->aborted = false; > fod->writedataactive = false; > @@ -556,12 +572,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc, > if (!nvmet_fc_tgt_a_get(assoc)) > goto out_free_queue; > > - queue->work_q = alloc_workqueue("ntfc%d.%d.%d", 0, 0, > - assoc->tgtport->fc_target_port.port_num, > - assoc->a_id, qid); > - if (!queue->work_q) > - goto out_a_put; > - > queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1]; > queue->qid = qid; > queue->sqsize = sqsize; > @@ -591,8 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc, > > out_fail_iodlist: > nvmet_fc_destroy_fcp_iodlist(assoc->tgtport, queue); > - destroy_workqueue(queue->work_q); > -out_a_put: > nvmet_fc_tgt_a_put(assoc); > out_free_queue: > kfree(queue); > @@ -615,8 +623,6 @@ nvmet_fc_tgt_queue_free(struct kref *ref) > > nvmet_fc_tgt_a_put(queue->assoc); > > - destroy_workqueue(queue->work_q); > - > kfree(queue); > } > > @@ -668,8 +674,6 @@ nvmet_fc_delete_target_queue(struct nvmet_fc_tgt_queue *queue) > } > spin_unlock_irqrestore(&queue->qlock, flags); > > - flush_workqueue(queue->work_q); > - > if (disconnect) > nvmet_sq_destroy(&queue->nvme_sq); > > @@ -1962,24 +1966,28 @@ nvmet_fc_fod_op_done(struct nvmet_fc_fcp_iod *fod) > } > > static void > -nvmet_fc_fcp_rqst_op_done_work(struct work_struct *work) > -{ > - struct nvmet_fc_fcp_iod *fod = > - container_of(work, struct nvmet_fc_fcp_iod, done_work); > - > - nvmet_fc_fod_op_done(fod); > -} > - > -static void > nvmet_fc_xmt_fcp_op_done(struct nvmefc_tgt_fcp_req *fcpreq) > { > struct nvmet_fc_fcp_iod *fod = fcpreq->nvmet_fc_private; > struct nvmet_fc_tgt_queue *queue = fod->queue; > - > - if (fod->tgtport->ops->target_features & NVMET_FCTGTFEAT_OPDONE_IN_ISR) > - /* context switch so completion is not in ISR context */ > - queue_work_on(queue->cpu, queue->work_q, &fod->done_work); > - else > + struct nvmet_fc_work_by_cpu *workcpu = nvmet_fc_workcpu(queue->cpu); > + unsigned long flags; > + bool running; > + > + if (fod->tgtport->ops->target_features & > + NVMET_FCTGTFEAT_OPDONE_IN_ISR) { > + /* context switch for processing */ > + > + spin_lock_irqsave(&workcpu->clock, flags); > + list_add_tail(&fod->work_list, &workcpu->fod_list); > + running = workcpu->running; > + workcpu->running = true; > + spin_unlock_irqrestore(&workcpu->clock, flags); > + > + if (!running) > + queue_work_on(workcpu->cpu, workcpu->work_q, > + &workcpu->cpu_work); > + } else > nvmet_fc_fod_op_done(fod); > } > > @@ -2069,6 +2077,7 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, > * layer until we have both based on csn. > */ > > + fod->started = true; > fod->fcpreq->done = nvmet_fc_xmt_fcp_op_done; > > fod->total_length = be32_to_cpu(cmdiu->data_len); > @@ -2144,19 +2153,6 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, > nvmet_fc_abort_op(tgtport, fod); > } > > -/* > - * Actual processing routine for received FC-NVME LS Requests from the LLD > - */ > -static void > -nvmet_fc_handle_fcp_rqst_work(struct work_struct *work) > -{ > - struct nvmet_fc_fcp_iod *fod = > - container_of(work, struct nvmet_fc_fcp_iod, work); > - struct nvmet_fc_tgtport *tgtport = fod->tgtport; > - > - nvmet_fc_handle_fcp_rqst(tgtport, fod); > -} > - > /** > * nvmet_fc_rcv_fcp_req - transport entry point called by an LLDD > * upon the reception of a NVME FCP CMD IU. > @@ -2186,6 +2182,9 @@ nvmet_fc_rcv_fcp_req(struct nvmet_fc_target_port *target_port, > struct nvme_fc_cmd_iu *cmdiu = cmdiubuf; > struct nvmet_fc_tgt_queue *queue; > struct nvmet_fc_fcp_iod *fod; > + struct nvmet_fc_work_by_cpu *workcpu; > + unsigned long flags; > + bool running; > > /* validate iu, so the connection id can be used to find the queue */ > if ((cmdiubuf_len != sizeof(*cmdiu)) || > @@ -2223,9 +2222,21 @@ nvmet_fc_rcv_fcp_req(struct nvmet_fc_target_port *target_port, > ((queue->qid - 1) % tgtport->ops->max_hw_queues) : 0; > memcpy(&fod->cmdiubuf, cmdiubuf, cmdiubuf_len); > > - if (tgtport->ops->target_features & NVMET_FCTGTFEAT_CMD_IN_ISR) > - queue_work_on(queue->cpu, queue->work_q, &fod->work); > - else > + if (tgtport->ops->target_features & NVMET_FCTGTFEAT_CMD_IN_ISR) { > + /* context switch for processing */ > + > + workcpu = nvmet_fc_workcpu(queue->cpu); > + > + spin_lock_irqsave(&workcpu->clock, flags); > + list_add_tail(&fod->work_list, &workcpu->fod_list); > + running = workcpu->running; > + workcpu->running = true; > + spin_unlock_irqrestore(&workcpu->clock, flags); > + > + if (!running) > + queue_work_on(workcpu->cpu, workcpu->work_q, > + &workcpu->cpu_work); > + } else > nvmet_fc_handle_fcp_rqst(tgtport, fod); > > return 0; > @@ -2391,13 +2402,17 @@ nvmet_fc_remove_port(struct nvmet_port *port) > { > struct nvmet_fc_tgtport *tgtport = port->priv; > unsigned long flags; > + bool matched = false; > > spin_lock_irqsave(&nvmet_fc_tgtlock, flags); > if (tgtport->port == port) { > - nvmet_fc_tgtport_put(tgtport); > + matched = true; > tgtport->port = NULL; > } > spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags); > + > + if (matched) > + nvmet_fc_tgtport_put(tgtport); > } > > static struct nvmet_fabrics_ops nvmet_fc_tgt_fcp_ops = { > @@ -2410,9 +2425,95 @@ static struct nvmet_fabrics_ops nvmet_fc_tgt_fcp_ops = { > .delete_ctrl = nvmet_fc_delete_ctrl, > }; > > +static void > +nvmet_fc_do_work_on_cpu(struct work_struct *work) > +{ > + struct nvmet_fc_work_by_cpu *workcpu = > + container_of(work, struct nvmet_fc_work_by_cpu, cpu_work); > + struct nvmet_fc_fcp_iod *fod; > + unsigned long flags; > + > + spin_lock_irqsave(&workcpu->clock, flags); > + > + fod = list_first_entry_or_null(&workcpu->fod_list, > + struct nvmet_fc_fcp_iod, work_list); > + for ( ; fod; ) { > + list_del(&fod->work_list); > + > + spin_unlock_irqrestore(&workcpu->clock, flags); > + > + if (fod->started) > + nvmet_fc_fod_op_done(fod); > + else > + nvmet_fc_handle_fcp_rqst(fod->tgtport, fod); > + > + spin_lock_irqsave(&workcpu->clock, flags); > + > + fod = list_first_entry_or_null(&workcpu->fod_list, > + struct nvmet_fc_fcp_iod, work_list); > + } > + > + workcpu->running = false; > + > + spin_unlock_irqrestore(&workcpu->clock, flags); > +} > + > +static int > +nvmet_fc_create_workqueues(void) > +{ > + struct nvmet_fc_work_by_cpu *workcpu; > + int i; > + > + nvmet_fc_cpu_cnt = num_active_cpus(); > + for (i = 0; i < nvmet_fc_cpu_cnt; i++, workcpu++) { > + workcpu = nvmet_fc_workcpu(i); > + > + workcpu->work_q = alloc_workqueue("nvmet-fc-cpu%d", 0, 0, i); > + if (!workcpu->work_q) > + return -ENOEXEC; > + workcpu->cpu = i; > + workcpu->running = false; > + spin_lock_init(&workcpu->clock); > + INIT_LIST_HEAD(&workcpu->fod_list); > + INIT_WORK(&workcpu->cpu_work, nvmet_fc_do_work_on_cpu); > + } > + > + return 0; > +} > + > +static void > +nvmet_fc_delete_workqueues(void) > +{ > + struct nvmet_fc_work_by_cpu *workcpu; > + int i; > + > + for (i = 0; i < nvmet_fc_cpu_cnt; i++, workcpu++) { > + workcpu = nvmet_fc_workcpu(i); > + > + /* sanity check - all work should be removed */ > + if (!list_empty(&workcpu->fod_list)) > + pr_warn("%s: cpu %d worklist not empty\n", __func__, i); > + > + if (workcpu->work_q) > + destroy_workqueue(workcpu->work_q); > + } > +} > + > static int __init nvmet_fc_init_module(void) > { > - return nvmet_register_transport(&nvmet_fc_tgt_fcp_ops); > + int ret; > + > + ret = nvmet_fc_create_workqueues(); > + if (ret) > + goto fail; > + > + ret = nvmet_register_transport(&nvmet_fc_tgt_fcp_ops); > + if (!ret) > + return 0; > + > +fail: > + nvmet_fc_delete_workqueues(); > + return -ENXIO; > } > > static void __exit nvmet_fc_exit_module(void) > @@ -2423,6 +2524,8 @@ static void __exit nvmet_fc_exit_module(void) > > nvmet_unregister_transport(&nvmet_fc_tgt_fcp_ops); > > + nvmet_fc_delete_workqueues(); > + > ida_destroy(&nvmet_fc_tgtport_cnt); > } > > Per-cpu ptr? Check drivers/scsi/fcoe/fcoe.c:fcoe_init() for reference. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare at suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N?rnberg)