Thanks Hannes for the review, please see my comments below, On 19/10/16 1:15 PM, "Hannes Reinecke" <hare@xxxxxxx> wrote: >On 10/19/2016 07:01 AM, manish.rangankar@xxxxxxxxxx wrote: >> From: Manish Rangankar <manish.rangankar@xxxxxxxxxx> >> >> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module >> for 41000 Series Converged Network Adapters by QLogic. >> >> This patch consists of following changes: >> - MAINTAINERS Makefile and Kconfig changes for qedi, >> - PCI driver registration, >> - iSCSI host level initialization, >> - Debugfs and log level infrastructure. >> >> Signed-off-by: Nilesh Javali <nilesh.javali@xxxxxxxxxx> >> Signed-off-by: Adheer Chandravanshi <adheer.chandravanshi@xxxxxxxxxx> >> Signed-off-by: Chad Dupuis <chad.dupuis@xxxxxxxxxx> >> Signed-off-by: Saurav Kashyap <saurav.kashyap@xxxxxxxxxx> >> Signed-off-by: Arun Easi <arun.easi@xxxxxxxxxx> >> Signed-off-by: Manish Rangankar <manish.rangankar@xxxxxxxxxx> >> --- >> MAINTAINERS | 6 + >> drivers/net/ethernet/qlogic/Kconfig | 12 - >> drivers/scsi/Kconfig | 1 + >> drivers/scsi/Makefile | 1 + >> drivers/scsi/qedi/Kconfig | 10 + >> drivers/scsi/qedi/Makefile | 5 + >> drivers/scsi/qedi/qedi.h | 286 +++++++ >> drivers/scsi/qedi/qedi_dbg.c | 143 ++++ >> drivers/scsi/qedi/qedi_dbg.h | 144 ++++ >> drivers/scsi/qedi/qedi_debugfs.c | 244 ++++++ >> drivers/scsi/qedi/qedi_hsi.h | 52 ++ >> drivers/scsi/qedi/qedi_main.c | 1550 >>+++++++++++++++++++++++++++++++++++ >> drivers/scsi/qedi/qedi_sysfs.c | 52 ++ >> drivers/scsi/qedi/qedi_version.h | 14 + >> 14 files changed, 2508 insertions(+), 12 deletions(-) >> create mode 100644 drivers/scsi/qedi/Kconfig >> create mode 100644 drivers/scsi/qedi/Makefile >> create mode 100644 drivers/scsi/qedi/qedi.h >> create mode 100644 drivers/scsi/qedi/qedi_dbg.c >> create mode 100644 drivers/scsi/qedi/qedi_dbg.h >> create mode 100644 drivers/scsi/qedi/qedi_debugfs.c >> create mode 100644 drivers/scsi/qedi/qedi_hsi.h >> create mode 100644 drivers/scsi/qedi/qedi_main.c >> create mode 100644 drivers/scsi/qedi/qedi_sysfs.c >> create mode 100644 drivers/scsi/qedi/qedi_version.h >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 5e925a2..906d05f 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -9909,6 +9909,12 @@ F: drivers/net/ethernet/qlogic/qed/ >> F: include/linux/qed/ >> F: drivers/net/ethernet/qlogic/qede/ >> >> +QLOGIC QL41xxx ISCSI DRIVER >> +M: QLogic-Storage-Upstream@xxxxxxxxxx >> +L: linux-scsi@xxxxxxxxxxxxxxx >> +S: Supported >> +F: drivers/scsi/qedi/ >> + >> QNX4 FILESYSTEM >> M: Anders Larsen <al@xxxxxxxxxxx> >> W: http://www.alarsen.net/linux/qnx4fs/ >> diff --git a/drivers/net/ethernet/qlogic/Kconfig >>b/drivers/net/ethernet/qlogic/Kconfig >> index bad4fae..28b4366 100644 >> --- a/drivers/net/ethernet/qlogic/Kconfig >> +++ b/drivers/net/ethernet/qlogic/Kconfig >> @@ -121,16 +121,4 @@ config INFINIBAND_QEDR >> config QED_ISCSI >> bool >> >> -config QEDI >> - tristate "QLogic QED 25/40/100Gb iSCSI driver" >> - depends on QED >> - select QED_LL2 >> - select QED_ISCSI >> - default n >> - ---help--- >> - This provides a temporary node that allows the compilation >> - and logical testing of the hardware offload iSCSI support >> - for QLogic QED. This would be replaced by the 'real' option >> - once the QEDI driver is added [+relocated]. >> - >> endif # NET_VENDOR_QLOGIC >Huh? You just introduce this one in patch 1/6. >Please fold them together so that this can be omitted. Yes, we will remove this in the next revision. -- snipped -- >> @@ -0,0 +1,52 @@ >> +/* >> + * QLogic iSCSI Offload Driver >> + * Copyright (c) 2016 Cavium Inc. >> + * >> + * This software is available under the terms of the GNU General >>Public License >> + * (GPL) Version 2, available from the file COPYING in the main >>directory of >> + * this source tree. >> + */ >> +#ifndef __QEDI_HSI__ >> +#define __QEDI_HSI__ >> +/********************************/ >> +/* Add include to common target */ >> +/********************************/ >> +#include <linux/qed/common_hsi.h> >> + >Please use kernel-doc style for comments Will do. --snipped-- >> +static void qedi_int_fp(struct qedi_ctx *qedi) >> +{ >> + struct qedi_fastpath *fp; >> + int id; >> + >> + memset((void *)qedi->fp_array, 0, MIN_NUM_CPUS_MSIX(qedi) * >> + sizeof(*qedi->fp_array)); >> + memset((void *)qedi->sb_array, 0, MIN_NUM_CPUS_MSIX(qedi) * >> + sizeof(*qedi->sb_array)); >> + >> + for (id = 0; id < MIN_NUM_CPUS_MSIX(qedi); id++) { >> + fp = &qedi->fp_array[id]; >> + fp->sb_info = &qedi->sb_array[id]; >> + fp->sb_id = id; >> + fp->qedi = qedi; >> + snprintf(fp->name, sizeof(fp->name), "%s-fp-%d", >> + "qedi", id); >> + >> + /* fp_array[i] ---- irq cookie >> + * So init data which is needed in int ctx >> + */ >> + } >> +} >> + >Please check if you cannot make use of Christophs irq rework. Sure, we will explore this. --snipped-- >> +static bool qedi_process_completions(struct qedi_fastpath *fp) >> +{ >> + struct qedi_work *qedi_work = NULL; >> + struct qedi_ctx *qedi = fp->qedi; >> + struct qed_sb_info *sb_info = fp->sb_info; >> + struct status_block *sb = sb_info->sb_virt; >> + struct qedi_percpu_s *p = NULL; >> + struct global_queue *que; >> + u16 prod_idx; >> + unsigned long flags; >> + union iscsi_cqe *cqe; >> + int cpu; >> + >> + /* Get the current firmware producer index */ >> + prod_idx = sb->pi_array[QEDI_PROTO_CQ_PROD_IDX]; >> + >> + if (prod_idx >= QEDI_CQ_SIZE) >> + prod_idx = prod_idx % QEDI_CQ_SIZE; >> + >> + que = qedi->global_queues[fp->sb_id]; >> + QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_IO, >> + "Before: global queue=%p prod_idx=%d cons_idx=%d, sb_id=%d\n", >> + que, prod_idx, que->cq_cons_idx, fp->sb_id); >> + >> + qedi->intr_cpu = fp->sb_id; >> + cpu = smp_processor_id(); >> + p = &per_cpu(qedi_percpu, cpu); >> + >> + if (unlikely(!p->iothread)) >> + WARN_ON(1); >> + >> + spin_lock_irqsave(&p->p_work_lock, flags); >> + while (que->cq_cons_idx != prod_idx) { >> + cqe = &que->cq[que->cq_cons_idx]; >> + >> + QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_IO, >> + "cqe=%p prod_idx=%d cons_idx=%d.\n", >> + cqe, prod_idx, que->cq_cons_idx); >> + >> + /* Alloc and copy to the cqe */ >> + qedi_work = kzalloc(sizeof(*qedi_work), GFP_ATOMIC); >> + if (qedi_work) { >> + INIT_LIST_HEAD(&qedi_work->list); >> + qedi_work->qedi = qedi; >> + memcpy(&qedi_work->cqe, cqe, sizeof(union iscsi_cqe)); >> + qedi_work->que_idx = fp->sb_id; >> + list_add_tail(&qedi_work->list, &p->work_list); >> + } else { >> + WARN_ON(1); >> + continue; >> + } >> + >Memory allocation in an interrupt routine? >You must be kidding ... We will revisit this code. > >> + que->cq_cons_idx++; >> + if (que->cq_cons_idx == QEDI_CQ_SIZE) >> + que->cq_cons_idx = 0; >> + } >> + wake_up_process(p->iothread); >> + spin_unlock_irqrestore(&p->p_work_lock, flags); >> + >> + return true; >> +} >> + >> +static bool qedi_fp_has_work(struct qedi_fastpath *fp) >> +{ >> + struct qedi_ctx *qedi = fp->qedi; >> + struct global_queue *que; >> + struct qed_sb_info *sb_info = fp->sb_info; >> + struct status_block *sb = sb_info->sb_virt; >> + u16 prod_idx; >> + >> + barrier(); >> + >> + /* Get the current firmware producer index */ >> + prod_idx = sb->pi_array[QEDI_PROTO_CQ_PROD_IDX]; >> + >> + /* Get the pointer to the global CQ this completion is on */ >> + que = qedi->global_queues[fp->sb_id]; >> + >> + /* prod idx wrap around uint16 */ >> + if (prod_idx >= QEDI_CQ_SIZE) >> + prod_idx = prod_idx % QEDI_CQ_SIZE; >> + >> + return (que->cq_cons_idx != prod_idx); >> +} >> + >> +/* MSI-X fastpath handler code */ >> +static irqreturn_t qedi_msix_handler(int irq, void *dev_id) >> +{ >> + struct qedi_fastpath *fp = dev_id; >> + struct qedi_ctx *qedi = fp->qedi; >> + bool wake_io_thread = true; >> + >> + qed_sb_ack(fp->sb_info, IGU_INT_DISABLE, 0); >> + >> +process_again: >> + wake_io_thread = qedi_process_completions(fp); >> + if (wake_io_thread) { >> + QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_DISC, >> + "process already running\n"); >> + } >> + >> + if (qedi_fp_has_work(fp) == 0) >> + qed_sb_update_sb_idx(fp->sb_info); >> + >> + /* Check for more work */ >> + rmb(); >> + >> + if (qedi_fp_has_work(fp) == 0) >> + qed_sb_ack(fp->sb_info, IGU_INT_ENABLE, 1); >> + else >> + goto process_again; >> + >> + return IRQ_HANDLED; >> +} >> + >> +/* simd handler for MSI/INTa */ >> +static void qedi_simd_int_handler(void *cookie) >> +{ >> + /* Cookie is qedi_ctx struct */ >> + struct qedi_ctx *qedi = (struct qedi_ctx *)cookie; >> + >> + QEDI_WARN(&qedi->dbg_ctx, "qedi=%p.\n", qedi); >> +} >> + >> +#define QEDI_SIMD_HANDLER_NUM 0 >> +static void qedi_sync_free_irqs(struct qedi_ctx *qedi) >> +{ >> + int i; >> + >> + if (qedi->int_info.msix_cnt) { >> + for (i = 0; i < qedi->int_info.used_cnt; i++) { >> + synchronize_irq(qedi->int_info.msix[i].vector); >> + irq_set_affinity_hint(qedi->int_info.msix[i].vector, >> + NULL); >> + free_irq(qedi->int_info.msix[i].vector, >> + &qedi->fp_array[i]); >> + } >> + } else { >> + qedi_ops->common->simd_handler_clean(qedi->cdev, >> + QEDI_SIMD_HANDLER_NUM); >> + } >> + >> + qedi->int_info.used_cnt = 0; >> + qedi_ops->common->set_fp_int(qedi->cdev, 0); >> +} >> + >Again, consider using the interrupt affinity rework from Christoph Hellwig Sure, we will explore this one also. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
