diff mbox series

[07/10] qla2xxx: edif - Reduce memory usage during low IO

Message ID 20221222043933.2825-8-njavali@marvell.com (mailing list archive)
State Accepted
Headers show
Series qla2xxx driver enhancements | expand

Commit Message

Nilesh Javali Dec. 22, 2022, 4:39 a.m. UTC 
From: Quinn Tran <qutran@marvell.com>

For edif, each IO require a secondary buffer to carry the
fcp cmnd. During high traffic time, these buffers are cached
in the qpair. As traffic dies down, these buffers will be
trimmed as needed. If traffic is reduce to none over 2 consecutive
intervals, then these buffers will be further trimmed.

Free fcp cmnd buffers to reduce memory usage
during slow IO time.

Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_def.h |  6 ++-
 drivers/scsi/qla2xxx/qla_gbl.h |  1 +
 drivers/scsi/qla2xxx/qla_mid.c | 94 ++++++++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_os.c  |  1 +
 4 files changed, 101 insertions(+), 1 deletion(-)

Comments

Quinn Tran Jan. 11, 2023, 4:53 p.m. UTC | #1 
+void __qla_adjust_buf(struct qla_qpair *qp)
+{
+ u32 trim;
+
+ qp->buf_pool.take_snapshot = 0;
+ qp->buf_pool.prev_max = qp->buf_pool.max_used;
+ qp->buf_pool.max_used = qp->buf_pool.num_active;
+
+ if (qp->buf_pool.prev_max > qp->buf_pool.max_used &&

You are assigning qp->buf_pool.prev_max with the value from qp->buf_pool.max_used a couple of lines above to max_used ... . This check looks incorrect because now the value for buf_pool.prev_max will be the same as buf_pool.max_used. 

Am I missing something?

QT:  Thanks for the review.    The 'qp->buf_pool.max_used = qp->buf_pool.num_active;'  line seems to be overlooked.  It changes max_used, where prev_max is not the same as max_used.
Himanshu Madhani Jan. 11, 2023, 6:30 p.m. UTC | #2 
> On Jan 11, 2023, at 8:53 AM, Quinn Tran <qutran@marvell.com> wrote:
> 
> +void __qla_adjust_buf(struct qla_qpair *qp)
> +{
> + u32 trim;
> +
> + qp->buf_pool.take_snapshot = 0;
> + qp->buf_pool.prev_max = qp->buf_pool.max_used;
> + qp->buf_pool.max_used = qp->buf_pool.num_active;
> +
> + if (qp->buf_pool.prev_max > qp->buf_pool.max_used &&
> 
> You are assigning qp->buf_pool.prev_max with the value from qp->buf_pool.max_used a couple of lines above to max_used ... . This check looks incorrect because now the value for buf_pool.prev_max will be the same as buf_pool.max_used. 
> 
> Am I missing something?
> 
> QT:  Thanks for the review.    The 'qp->buf_pool.max_used = qp->buf_pool.num_active;'  line seems to be overlooked.  It changes max_used, where prev_max is not the same as max_used.
> 

Indeed. Thanks for the clarification. Rest of the patch looks good. 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 6f6190404939..972f1144b9d3 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3735,7 +3735,10 @@  struct  qla_buf_pool {
 	u16 num_bufs;
 	u16 num_active;
 	u16 max_used;
-	u16 reserved;
+	u16 num_alloc;
+	u16 prev_max;
+	u16 pad;
+	uint32_t take_snapshot:1;
 	unsigned long *buf_map;
 	void **buf_array;
 	dma_addr_t *dma_array;
@@ -4874,6 +4877,7 @@  typedef struct scsi_qla_host {
 #define LOOP_READY	5
 #define LOOP_DEAD	6
 
+	unsigned long   buf_expired;
 	unsigned long   relogin_jif;
 	unsigned long   dpc_flags;
 #define RESET_MARKER_NEEDED	0	/* Send marker to ISP. */
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index d802d37fe739..9142df876c73 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -293,6 +293,7 @@  extern void qla2x00_alert_all_vps(struct rsp_que *, uint16_t *);
 extern void qla2x00_async_event(scsi_qla_host_t *, struct rsp_que *,
 	uint16_t *);
 extern int  qla2x00_vp_abort_isp(scsi_qla_host_t *);
+void qla_adjust_buf(struct scsi_qla_host *);
 
 /*
  * Global Function Prototypes in qla_iocb.c source file.
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 5976a2f036e6..c6ca39b8e23d 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -1170,6 +1170,7 @@  int qla_get_buf(struct scsi_qla_host *vha, struct qla_qpair *qp, struct qla_buf_
 
 		dsc->buf = qp->buf_pool.buf_array[tag] = buf;
 		dsc->buf_dma = qp->buf_pool.dma_array[tag] = buf_dma;
+		qp->buf_pool.num_alloc++;
 	} else {
 		dsc->buf = qp->buf_pool.buf_array[tag];
 		dsc->buf_dma = qp->buf_pool.dma_array[tag];
@@ -1185,14 +1186,107 @@  int qla_get_buf(struct scsi_qla_host *vha, struct qla_qpair *qp, struct qla_buf_
 	return 0;
 }
 
+void qla_trim_buf(struct qla_qpair *qp, u16 trim)
+{
+	int i, j;
+	struct qla_hw_data *ha = qp->vha->hw;
+
+	if (!trim)
+		return;
+
+	for (i = 0; i < trim; i++) {
+		j = qp->buf_pool.num_alloc - 1;
+		if (test_bit(j, qp->buf_pool.buf_map)) {
+			ql_dbg(ql_dbg_io + ql_dbg_verbose, qp->vha, 0x300b,
+			       "QP id(%d): trim active buf[%d]. Remain %d bufs\n",
+			       qp->id, j, qp->buf_pool.num_alloc);
+			return;
+		}
+
+		if (qp->buf_pool.buf_array[j]) {
+			dma_pool_free(ha->fcp_cmnd_dma_pool, qp->buf_pool.buf_array[j],
+				      qp->buf_pool.dma_array[j]);
+			qp->buf_pool.buf_array[j] = NULL;
+			qp->buf_pool.dma_array[j] = 0;
+		}
+		qp->buf_pool.num_alloc--;
+		if (!qp->buf_pool.num_alloc)
+			break;
+	}
+	ql_dbg(ql_dbg_io + ql_dbg_verbose, qp->vha, 0x3010,
+	       "QP id(%d): trimmed %d bufs. Remain %d bufs\n",
+	       qp->id, trim, qp->buf_pool.num_alloc);
+}
+
+void __qla_adjust_buf(struct qla_qpair *qp)
+{
+	u32 trim;
+
+	qp->buf_pool.take_snapshot = 0;
+	qp->buf_pool.prev_max = qp->buf_pool.max_used;
+	qp->buf_pool.max_used = qp->buf_pool.num_active;
+
+	if (qp->buf_pool.prev_max > qp->buf_pool.max_used &&
+	    qp->buf_pool.num_alloc > qp->buf_pool.max_used) {
+		/* down trend */
+		trim = qp->buf_pool.num_alloc - qp->buf_pool.max_used;
+		trim  = (trim * 10) / 100;
+		trim = trim ? trim : 1;
+		qla_trim_buf(qp, trim);
+	} else if (!qp->buf_pool.prev_max  && !qp->buf_pool.max_used) {
+		/* 2 periods of no io */
+		qla_trim_buf(qp, qp->buf_pool.num_alloc);
+	}
+}
 
 /* it is assume qp->qp_lock is held at this point */
 void qla_put_buf(struct qla_qpair *qp, struct qla_buf_dsc *dsc)
 {
 	if (dsc->tag == TAG_FREED)
 		return;
+	lockdep_assert_held(qp->qp_lock_ptr);
 
 	clear_bit(dsc->tag, qp->buf_pool.buf_map);
 	qp->buf_pool.num_active--;
 	dsc->tag = TAG_FREED;
+
+	if (qp->buf_pool.take_snapshot)
+		__qla_adjust_buf(qp);
+}
+
+#define EXPIRE (60 * HZ)
+void qla_adjust_buf(struct scsi_qla_host *vha)
+{
+	unsigned long flags;
+	int i;
+	struct qla_qpair *qp;
+
+	if (vha->vp_idx)
+		return;
+
+	if (!vha->buf_expired) {
+		vha->buf_expired = jiffies + EXPIRE;
+		return;
+	}
+	if (time_before(jiffies, vha->buf_expired))
+		return;
+
+	vha->buf_expired = jiffies + EXPIRE;
+
+	for (i = 0; i < vha->hw->num_qpairs; i++) {
+		qp = vha->hw->queue_pair_map[i];
+		if (!qp)
+			continue;
+		if (!qp->buf_pool.num_alloc)
+			continue;
+
+		if (qp->buf_pool.take_snapshot) {
+			/* no io has gone through in the last EXPIRE period */
+			spin_lock_irqsave(qp->qp_lock_ptr, flags);
+			__qla_adjust_buf(qp);
+			spin_unlock_irqrestore(qp->qp_lock_ptr, flags);
+		} else {
+			qp->buf_pool.take_snapshot = 1;
+		}
+	}
 }
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index f8758cea11d6..d07a914559d3 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -7522,6 +7522,7 @@  qla2x00_timer(struct timer_list *t)
 		set_bit(SET_ZIO_THRESHOLD_NEEDED, &vha->dpc_flags);
 		start_dpc++;
 	}
+	qla_adjust_buf(vha);
 
 	/* borrowing w to signify dpc will run */
 	w = 0;