Re: [PATCH 6/8] V2 ipr: Implement block iopoll

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Quoting Asias He <asias.hejun@xxxxxxxxx>:

Hello Wen Xiong,

On Sat, Jan 12, 2013 at 7:43 AM,  <wenxiong@xxxxxxxxxxxxxxxxxx> wrote:
This patch implements blk iopoll in ipr driver for performance improvement.

Can you provide the performance numbers with/without the io polling?
It would be interesting to know.


we enable blk iopoll support in ipr driver when firmware supports multiple hrrq. With multiple hrrq support and cpu binding tuning, we can see 10%-20% performance improvement in some FIO cases.

Thanks,
Wendy

Signed-off-by: Wen Xiong <wenxiong@xxxxxxxxxxxxxxxxxx>
---
drivers/scsi/ipr.c | 221 +++++++++++++++++++++++++++++++++++++++++------------
 drivers/scsi/ipr.h |    6 +
 2 files changed, 178 insertions(+), 49 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c        2013-01-11 16:11:20.062476228 -0600
+++ b/drivers/scsi/ipr.c        2013-01-11 16:11:50.275910225 -0600
@@ -108,6 +108,7 @@ static const struct ipr_chip_cfg_t ipr_c
                .max_cmds = 100,
                .cache_line_size = 0x20,
                .clear_isr = 1,
+               .iopoll_weight = 0,
                {
                        .set_interrupt_mask_reg = 0x0022C,
                        .clr_interrupt_mask_reg = 0x00230,
@@ -132,6 +133,7 @@ static const struct ipr_chip_cfg_t ipr_c
                .max_cmds = 100,
                .cache_line_size = 0x20,
                .clear_isr = 1,
+               .iopoll_weight = 0,
                {
                        .set_interrupt_mask_reg = 0x00288,
                        .clr_interrupt_mask_reg = 0x0028C,
@@ -156,6 +158,7 @@ static const struct ipr_chip_cfg_t ipr_c
                .max_cmds = 1000,
                .cache_line_size = 0x20,
                .clear_isr = 0,
+               .iopoll_weight = 64,
                {
                        .set_interrupt_mask_reg = 0x00010,
                        .clr_interrupt_mask_reg = 0x00018,
@@ -3560,6 +3563,95 @@ static struct device_attribute ipr_ioa_r
        .store = ipr_store_reset_adapter
 };

+static int ipr_iopoll(struct blk_iopoll *iop, int budget);
+ /**
+ * ipr_show_iopoll_weight - Show ipr polling mode
+ * @dev:       class device struct
+ * @buf:       buffer
+ *
+ * Return value:
+ *     number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_iopoll_weight(struct device *dev,
+                                  struct device_attribute *attr, char *buf)
+{
+       struct Scsi_Host *shost = class_to_shost(dev);
+       struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+       unsigned long lock_flags = 0;
+       int len;
+
+       spin_lock_irqsave(shost->host_lock, lock_flags);
+       len = snprintf(buf, PAGE_SIZE, "%d\n", ioa_cfg->iopoll_weight);
+       spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+       return len;
+}
+
+/**
+ * ipr_store_iopoll_weight - Change the adapter's polling mode
+ * @dev:       class device struct
+ * @buf:       buffer
+ *
+ * Return value:
+ *     number of bytes printed to buffer
+ **/
+static ssize_t ipr_store_iopoll_weight(struct device *dev,
+                                       struct device_attribute *attr,
+                                       const char *buf, size_t count)
+{
+       struct Scsi_Host *shost = class_to_shost(dev);
+       struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata;
+       unsigned long user_iopoll_weight;
+       unsigned long lock_flags = 0;
+       int i;
+
+       if (!ioa_cfg->sis64) {
+ dev_info(&ioa_cfg->pdev->dev, "blk-iopoll not supported on this adapter\n");
+               return -EINVAL;
+       }
+       if (kstrtoul(buf, 10, &user_iopoll_weight))
+               return -EINVAL;
+
+       if (user_iopoll_weight > 256) {
+ dev_info(&ioa_cfg->pdev->dev, "Invalid blk-iopoll weight. It must be less than 256\n");
+               return -EINVAL;
+       }
+
+       if (user_iopoll_weight == ioa_cfg->iopoll_weight) {
+ dev_info(&ioa_cfg->pdev->dev, "Current blk-iopoll weight has the same weight\n");
+               return strlen(buf);
+       }
+
+       if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+                       ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+               for (i = 1; i < ioa_cfg->hrrq_num; i++)
+                       blk_iopoll_disable(&ioa_cfg->hrrq[i].iopoll);
+       }
+
+       spin_lock_irqsave(shost->host_lock, lock_flags);
+       ioa_cfg->iopoll_weight = user_iopoll_weight;
+       if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+                       ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+               for (i = 1; i < ioa_cfg->hrrq_num; i++) {
+                       blk_iopoll_init(&ioa_cfg->hrrq[i].iopoll,
+                                       ioa_cfg->iopoll_weight, ipr_iopoll);
+                       blk_iopoll_enable(&ioa_cfg->hrrq[i].iopoll);
+               }
+       }
+       spin_unlock_irqrestore(shost->host_lock, lock_flags);
+
+       return strlen(buf);
+}
+
+static struct device_attribute ipr_iopoll_weight_attr = {
+       .attr = {
+               .name =         "iopoll_weight",
+               .mode =         S_IRUGO | S_IWUSR,
+       },
+       .show = ipr_show_iopoll_weight,
+       .store = ipr_store_iopoll_weight
+};
+
 /**
  * ipr_alloc_ucode_buffer - Allocates a microcode download buffer
  * @buf_len:           buffer length
@@ -3928,6 +4020,7 @@ static struct device_attribute *ipr_ioa_
        &ipr_ioa_reset_attr,
        &ipr_update_fw_attr,
        &ipr_ioa_fw_type_attr,
+       &ipr_iopoll_weight_attr,
        NULL,
 };

@@ -5218,7 +5311,7 @@ static void ipr_isr_eh(struct ipr_ioa_cf
        ipr_initiate_ioa_reset(ioa_cfg, IPR_SHUTDOWN_NONE);
 }

-static int __ipr_process_hrrq(struct ipr_hrr_queue *hrr_queue,
+static int ipr_process_hrrq(struct ipr_hrr_queue *hrr_queue, int budget,
                                                struct list_head *doneq)
 {
        u32 ioasc;
@@ -5260,9 +5353,41 @@ static int __ipr_process_hrrq(struct ipr
                        hrr_queue->toggle_bit ^= 1u;
                }
                num_hrrq++;
+               if (budget > 0 && num_hrrq >= budget)
+                       break;
        }
+
        return num_hrrq;
 }
+
+static int ipr_iopoll(struct blk_iopoll *iop, int budget)
+{
+       struct ipr_ioa_cfg *ioa_cfg;
+       struct ipr_hrr_queue *hrrq;
+       struct ipr_cmnd *ipr_cmd, *temp;
+       unsigned long hrrq_flags;
+       int completed_ops;
+       LIST_HEAD(doneq);
+
+       hrrq = container_of(iop, struct ipr_hrr_queue, iopoll);
+       ioa_cfg = hrrq->ioa_cfg;
+
+       spin_lock_irqsave(hrrq->lock, hrrq_flags);
+       completed_ops = ipr_process_hrrq(hrrq, budget, &doneq);
+
+       if (completed_ops < budget)
+               blk_iopoll_complete(iop);
+       spin_unlock_irqrestore(hrrq->lock, hrrq_flags);
+
+       list_for_each_entry_safe(ipr_cmd, temp, &doneq, queue) {
+               list_del(&ipr_cmd->queue);
+               del_timer(&ipr_cmd->timer);
+               ipr_cmd->fast_done(ipr_cmd);
+       }
+
+       return completed_ops;
+}
+
 /**
  * ipr_isr - Interrupt service routine
  * @irq:       irq number
@@ -5277,8 +5402,6 @@ static irqreturn_t ipr_isr(int irq, void
        struct ipr_ioa_cfg *ioa_cfg = hrrq->ioa_cfg;
        unsigned long hrrq_flags = 0;
        u32 int_reg = 0;
-       u32 ioasc;
-       u16 cmd_index;
        int num_hrrq = 0;
        int irq_none = 0;
        struct ipr_cmnd *ipr_cmd, *temp;
@@ -5293,60 +5416,30 @@ static irqreturn_t ipr_isr(int irq, void
        }

        while (1) {
-               ipr_cmd = NULL;
-
- while ((be32_to_cpu(*hrrq->hrrq_curr) & IPR_HRRQ_TOGGLE_BIT) ==
-                      hrrq->toggle_bit) {
-
-                       cmd_index = (be32_to_cpu(*hrrq->hrrq_curr) &
- IPR_HRRQ_REQ_RESP_HANDLE_MASK) >> IPR_HRRQ_REQ_RESP_HANDLE_SHIFT;
-
-                       if (unlikely(cmd_index > hrrq->max_cmd_id ||
-                                    cmd_index < hrrq->min_cmd_id)) {
-                               ipr_isr_eh(ioa_cfg,
- "Invalid response handle from IOA: ",
-                                       cmd_index);
-                               rc = IRQ_HANDLED;
-                               goto unlock_out;
-                       }
-
-                       ipr_cmd = ioa_cfg->ipr_cmnd_list[cmd_index];
-                       ioasc = be32_to_cpu(ipr_cmd->s.ioasa.hdr.ioasc);
-
-                       ipr_trc_hook(ipr_cmd, IPR_TRACE_FINISH, ioasc);
-
-                       list_move_tail(&ipr_cmd->queue, &doneq);
-
-                       rc = IRQ_HANDLED;
-
-                       if (hrrq->hrrq_curr < hrrq->hrrq_end) {
-                               hrrq->hrrq_curr++;
-                       } else {
-                               hrrq->hrrq_curr = hrrq->hrrq_start;
-                               hrrq->toggle_bit ^= 1u;
-                       }
-               }
+               if (ipr_process_hrrq(hrrq, -1, &doneq)) {
+                       rc =  IRQ_HANDLED;

-               if (ipr_cmd && !ioa_cfg->clear_isr)
-                       break;
+                       if (!ioa_cfg->clear_isr)
+                               break;

-               if (ipr_cmd != NULL) {
                        /* Clear the PCI interrupt */
                        num_hrrq = 0;
                        do {
- writel(IPR_PCII_HRRQ_UPDATED, ioa_cfg->regs.clr_interrupt_reg32);
+                               writel(IPR_PCII_HRRQ_UPDATED,
+                                    ioa_cfg->regs.clr_interrupt_reg32);
int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32);
                        } while (int_reg & IPR_PCII_HRRQ_UPDATED &&
-                                       num_hrrq++ < IPR_MAX_HRRQ_RETRIES);
+                               num_hrrq++ < IPR_MAX_HRRQ_RETRIES);

                } else if (rc == IRQ_NONE && irq_none == 0) {
int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32);
                        irq_none++;
                } else if (num_hrrq == IPR_MAX_HRRQ_RETRIES &&
                           int_reg & IPR_PCII_HRRQ_UPDATED) {
- ipr_isr_eh(ioa_cfg, "Error clearing HRRQ: ", num_hrrq);
+                       ipr_isr_eh(ioa_cfg,
+                               "Error clearing HRRQ: ", num_hrrq);
                        rc = IRQ_HANDLED;
-                       goto unlock_out;
+                       break;
                } else
                        break;
        }
@@ -5354,7 +5447,6 @@ static irqreturn_t ipr_isr(int irq, void
        if (unlikely(rc == IRQ_NONE))
                rc = ipr_handle_other_interrupt(ioa_cfg, int_reg);

-unlock_out:
        spin_unlock_irqrestore(hrrq->lock, hrrq_flags);
        list_for_each_entry_safe(ipr_cmd, temp, &doneq, queue) {
                list_del(&ipr_cmd->queue);
@@ -5375,6 +5467,7 @@ unlock_out:
 static irqreturn_t ipr_isr_mhrrq(int irq, void *devp)
 {
        struct ipr_hrr_queue *hrrq = (struct ipr_hrr_queue *)devp;
+       struct ipr_ioa_cfg *ioa_cfg = hrrq->ioa_cfg;
        unsigned long hrrq_flags = 0;
        struct ipr_cmnd *ipr_cmd, *temp;
        irqreturn_t rc = IRQ_NONE;
@@ -5388,11 +5481,22 @@ static irqreturn_t ipr_isr_mhrrq(int irq
                return IRQ_NONE;
        }

-       if ((be32_to_cpu(*hrrq->hrrq_curr) & IPR_HRRQ_TOGGLE_BIT) ==
-            hrrq->toggle_bit)
+       if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+                       ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+               if ((be32_to_cpu(*hrrq->hrrq_curr) & IPR_HRRQ_TOGGLE_BIT) ==
+                      hrrq->toggle_bit) {
+                       if (!blk_iopoll_sched_prep(&hrrq->iopoll))
+                               blk_iopoll_sched(&hrrq->iopoll);
+                       spin_unlock_irqrestore(hrrq->lock, hrrq_flags);
+                       return IRQ_HANDLED;
+               }
+       } else {
+               if ((be32_to_cpu(*hrrq->hrrq_curr) & IPR_HRRQ_TOGGLE_BIT) ==
+                       hrrq->toggle_bit)

-               if (__ipr_process_hrrq(hrrq, &doneq))
-                       rc =  IRQ_HANDLED;
+                       if (ipr_process_hrrq(hrrq, -1, &doneq))
+                               rc =  IRQ_HANDLED;
+       }

        spin_unlock_irqrestore(hrrq->lock, hrrq_flags);

@@ -9690,7 +9794,7 @@ static void ipr_remove(struct pci_dev *p
static int ipr_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
 {
        struct ipr_ioa_cfg *ioa_cfg;
-       int rc;
+       int rc, i;

        rc = ipr_probe_ioa(pdev, dev_id);

@@ -9737,6 +9841,17 @@ static int ipr_probe(struct pci_dev *pde
scsi_add_device(ioa_cfg->host, IPR_IOA_BUS, IPR_IOA_TARGET, IPR_IOA_LUN);
        ioa_cfg->allow_ml_add_del = 1;
        ioa_cfg->host->max_channel = IPR_VSET_BUS;
+       ioa_cfg->iopoll_weight = ioa_cfg->chip_cfg->iopoll_weight;
+
+       if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+                       ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+               for (i = 1; i < ioa_cfg->hrrq_num; i++) {
+                       blk_iopoll_init(&ioa_cfg->hrrq[i].iopoll,
+                                       ioa_cfg->iopoll_weight, ipr_iopoll);
+                       blk_iopoll_enable(&ioa_cfg->hrrq[i].iopoll);
+               }
+       }
+
        schedule_work(&ioa_cfg->work_q);
        return 0;
 }
@@ -9755,8 +9870,16 @@ static void ipr_shutdown(struct pci_dev
 {
        struct ipr_ioa_cfg *ioa_cfg = pci_get_drvdata(pdev);
        unsigned long lock_flags = 0;
+       int i;

        spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+       if (blk_iopoll_enabled && ioa_cfg->iopoll_weight &&
+                       ioa_cfg->sis64 && ioa_cfg->nvectors > 1) {
+               ioa_cfg->iopoll_weight = 0;
+               for (i = 1; i < ioa_cfg->hrrq_num; i++)
+                       blk_iopoll_disable(&ioa_cfg->hrrq[i].iopoll);
+       }
+
        while (ioa_cfg->in_reset_reload) {
spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); wait_event(ioa_cfg->reset_wait_q, !ioa_cfg->in_reset_reload);
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h        2013-01-11 16:10:56.114660090 -0600
+++ b/drivers/scsi/ipr.h        2013-01-11 16:11:50.275910225 -0600
@@ -32,6 +32,7 @@
 #include <linux/libata.h>
 #include <linux/list.h>
 #include <linux/kref.h>
+#include <linux/blk-iopoll.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>

@@ -492,6 +493,8 @@ struct ipr_hrr_queue {
        u8 allow_interrupts:1;
        u8 ioa_is_dead:1;
        u8 allow_cmds:1;
+
+       struct blk_iopoll iopoll;
 };

 /* Command packet structure */
@@ -1348,6 +1351,7 @@ struct ipr_chip_cfg_t {
        u16 max_cmds;
        u8 cache_line_size;
        u8 clear_isr;
+       u32 iopoll_weight;
        struct ipr_interrupt_offsets regs;
 };

@@ -1534,6 +1538,8 @@ struct ipr_ioa_cfg {
                char desc[22];
        } vectors_info[IPR_MAX_MSIX_VECTORS];

+       u32 iopoll_weight;
+
 }; /* struct ipr_ioa_cfg */

 struct ipr_cmnd {

--
--
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



--
Asias



--
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


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux