[PATCH net 0/3] ionic: memory usage fixes

[PATCH net 0/3] ionic: memory usage fixes

[Shannon Nelson]

This patchset addresses some memory leaks and incorrect
io reads.

Shannon Nelson (3):
  ionic: clean up sparse complaints
  ionic: no rx flush in deinit
  ionic: fix mem leak in rx_empty

 .../net/ethernet/pensando/ionic/ionic_dev.c   |  4 +-
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  2 +
 .../net/ethernet/pensando/ionic/ionic_fw.c    |  6 +--
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 29 ++++++------
 .../net/ethernet/pensando/ionic/ionic_main.c  |  4 +-
 .../net/ethernet/pensando/ionic/ionic_stats.h |  2 +-
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 47 +++++++------------
 .../net/ethernet/pensando/ionic/ionic_txrx.h  |  1 -
 8 files changed, 40 insertions(+), 55 deletions(-)

-- 
2.17.1

[PATCH net 1/3] ionic: clean up sparse complaints

[Shannon Nelson]

The sparse complaints around the static_asserts were obscuring
more useful complaints.  So, don't check the static_asserts,
and fix the remaining sparse complaints.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_dev.c   |  4 +--
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  2 ++
 .../net/ethernet/pensando/ionic/ionic_fw.c    |  6 ++--
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 28 +++++++++----------
 .../net/ethernet/pensando/ionic/ionic_main.c  |  4 +--
 .../net/ethernet/pensando/ionic/ionic_stats.h |  2 +-
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 10 +++----
 7 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index 545c99b15df8..dc5fbc2704f3 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -333,7 +333,7 @@ int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data)
 	union ionic_dev_cmd cmd = {
 		.vf_setattr.opcode = IONIC_CMD_VF_SETATTR,
 		.vf_setattr.attr = attr,
-		.vf_setattr.vf_index = vf,
+		.vf_setattr.vf_index = cpu_to_le16(vf),
 	};
 	int err;
 
@@ -391,7 +391,7 @@ void ionic_dev_cmd_queue_identify(struct ionic_dev *idev,
 {
 	union ionic_dev_cmd cmd = {
 		.q_identify.opcode = IONIC_CMD_Q_IDENTIFY,
-		.q_identify.lif_type = lif_type,
+		.q_identify.lif_type = cpu_to_le16(lif_type),
 		.q_identify.type = qtype,
 		.q_identify.ver = qver,
 	};
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index c109cd5a0471..6c243b17312c 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -29,6 +29,7 @@ struct ionic_dev_bar {
 	int res_index;
 };
 
+#ifndef __CHECKER__
 /* Registers */
 static_assert(sizeof(struct ionic_intr) == 32);
 
@@ -119,6 +120,7 @@ static_assert(sizeof(struct ionic_vf_setattr_cmd) == 64);
 static_assert(sizeof(struct ionic_vf_setattr_comp) == 16);
 static_assert(sizeof(struct ionic_vf_getattr_cmd) == 64);
 static_assert(sizeof(struct ionic_vf_getattr_comp) == 16);
+#endif /* __CHECKER__ */
 
 struct ionic_devinfo {
 	u8 asic_type;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_fw.c b/drivers/net/ethernet/pensando/ionic/ionic_fw.c
index f492ae406a60..d7bbf336c6f6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_fw.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_fw.c
@@ -27,9 +27,9 @@ static void ionic_dev_cmd_firmware_download(struct ionic_dev *idev, u64 addr,
 {
 	union ionic_dev_cmd cmd = {
 		.fw_download.opcode = IONIC_CMD_FW_DOWNLOAD,
-		.fw_download.offset = offset,
-		.fw_download.addr = addr,
-		.fw_download.length = length
+		.fw_download.offset = cpu_to_le32(offset),
+		.fw_download.addr = cpu_to_le64(addr),
+		.fw_download.length = cpu_to_le32(length),
 	};
 
 	ionic_dev_cmd_go(idev, &cmd);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index d655a7ae3058..591c644b8e69 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1915,11 +1915,11 @@ static int ionic_get_vf_config(struct net_device *netdev,
 		ret = -EINVAL;
 	} else {
 		ivf->vf           = vf;
-		ivf->vlan         = ionic->vfs[vf].vlanid;
+		ivf->vlan         = le16_to_cpu(ionic->vfs[vf].vlanid);
 		ivf->qos	  = 0;
 		ivf->spoofchk     = ionic->vfs[vf].spoofchk;
 		ivf->linkstate    = ionic->vfs[vf].linkstate;
-		ivf->max_tx_rate  = ionic->vfs[vf].maxrate;
+		ivf->max_tx_rate  = le32_to_cpu(ionic->vfs[vf].maxrate);
 		ivf->trusted      = ionic->vfs[vf].trusted;
 		ether_addr_copy(ivf->mac, ionic->vfs[vf].macaddr);
 	}
@@ -2019,7 +2019,7 @@ static int ionic_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
 		ret = ionic_set_vf_config(ionic, vf,
 					  IONIC_VF_ATTR_VLAN, (u8 *)&vlan);
 		if (!ret)
-			ionic->vfs[vf].vlanid = vlan;
+			ionic->vfs[vf].vlanid = cpu_to_le16(vlan);
 	}
 
 	up_write(&ionic->vf_op_lock);
@@ -2048,7 +2048,7 @@ static int ionic_set_vf_rate(struct net_device *netdev, int vf,
 		ret = ionic_set_vf_config(ionic, vf,
 					  IONIC_VF_ATTR_RATE, (u8 *)&tx_max);
 		if (!ret)
-			lif->ionic->vfs[vf].maxrate = tx_max;
+			lif->ionic->vfs[vf].maxrate = cpu_to_le32(tx_max);
 	}
 
 	up_write(&ionic->vf_op_lock);
@@ -2981,14 +2981,14 @@ void ionic_lif_unregister(struct ionic_lif *lif)
 
 static void ionic_lif_queue_identify(struct ionic_lif *lif)
 {
+	union ionic_q_identity __iomem *q_ident;
 	struct ionic *ionic = lif->ionic;
-	union ionic_q_identity *q_ident;
 	struct ionic_dev *idev;
 	int qtype;
 	int err;
 
 	idev = &lif->ionic->idev;
-	q_ident = (union ionic_q_identity *)&idev->dev_cmd_regs->data;
+	q_ident = (union ionic_q_identity __iomem *)&idev->dev_cmd_regs->data;
 
 	for (qtype = 0; qtype < ARRAY_SIZE(ionic_qtype_versions); qtype++) {
 		struct ionic_qtype_info *qti = &lif->qtype_info[qtype];
@@ -3011,14 +3011,14 @@ static void ionic_lif_queue_identify(struct ionic_lif *lif)
 					     ionic_qtype_versions[qtype]);
 		err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
 		if (!err) {
-			qti->version   = q_ident->version;
-			qti->supported = q_ident->supported;
-			qti->features  = le64_to_cpu(q_ident->features);
-			qti->desc_sz   = le16_to_cpu(q_ident->desc_sz);
-			qti->comp_sz   = le16_to_cpu(q_ident->comp_sz);
-			qti->sg_desc_sz   = le16_to_cpu(q_ident->sg_desc_sz);
-			qti->max_sg_elems = le16_to_cpu(q_ident->max_sg_elems);
-			qti->sg_desc_stride = le16_to_cpu(q_ident->sg_desc_stride);
+			qti->version   = readb(&q_ident->version);
+			qti->supported = readb(&q_ident->supported);
+			qti->features  = readq(&q_ident->features);
+			qti->desc_sz   = readw(&q_ident->desc_sz);
+			qti->comp_sz   = readw(&q_ident->comp_sz);
+			qti->sg_desc_sz   = readw(&q_ident->sg_desc_sz);
+			qti->max_sg_elems = readw(&q_ident->max_sg_elems);
+			qti->sg_desc_stride = readw(&q_ident->sg_desc_stride);
 		}
 		mutex_unlock(&ionic->dev_cmd_lock);
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index ee0740881af3..d355676f6c16 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -311,7 +311,7 @@ int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
 
 static void ionic_dev_cmd_clean(struct ionic *ionic)
 {
-	union ionic_dev_cmd_regs *regs = ionic->idev.dev_cmd_regs;
+	union __iomem ionic_dev_cmd_regs *regs = ionic->idev.dev_cmd_regs;
 
 	iowrite32(0, &regs->doorbell);
 	memset_io(&regs->cmd, 0, sizeof(regs->cmd));
@@ -333,7 +333,7 @@ int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds)
 	 */
 	max_wait = jiffies + (max_seconds * HZ);
 try_again:
-	opcode = idev->dev_cmd_regs->cmd.cmd.opcode;
+	opcode = readb(&idev->dev_cmd_regs->cmd.cmd.opcode);
 	start_time = jiffies;
 	do {
 		done = ionic_dev_cmd_done(idev);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.h b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
index 3f543512616e..2a725834f792 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_stats.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
@@ -49,7 +49,7 @@ extern const int ionic_num_stats_grps;
 	(*((u64 *)(((u8 *)(base_ptr)) + (desc_ptr)->offset)))
 
 #define IONIC_READ_STAT_LE64(base_ptr, desc_ptr) \
-	__le64_to_cpu(*((u64 *)(((u8 *)(base_ptr)) + (desc_ptr)->offset)))
+	__le64_to_cpu(*((__le64 *)(((u8 *)(base_ptr)) + (desc_ptr)->offset)))
 
 struct ionic_stat_desc {
 	char name[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 169ac4f54640..8f6fc7142bc5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -200,7 +200,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
 	if (likely(netdev->features & NETIF_F_RXCSUM)) {
 		if (comp->csum_flags & IONIC_RXQ_COMP_CSUM_F_CALC) {
 			skb->ip_summed = CHECKSUM_COMPLETE;
-			skb->csum = (__wsum)le16_to_cpu(comp->csum);
+			skb->csum = (__force __wsum)le16_to_cpu(comp->csum);
 			stats->csum_complete++;
 		}
 	} else {
@@ -812,6 +812,7 @@ static int ionic_tx_tso(struct ionic_queue *q, struct sk_buff *skb)
 	skb_frag_t *frag;
 	bool start, done;
 	bool outer_csum;
+	dma_addr_t addr;
 	bool has_vlan;
 	u16 desc_len;
 	u8 desc_nsge;
@@ -893,11 +894,10 @@ static int ionic_tx_tso(struct ionic_queue *q, struct sk_buff *skb)
 			if (frag_left > 0) {
 				len = min(frag_left, left);
 				frag_left -= len;
-				elem->addr =
-				    cpu_to_le64(ionic_tx_map_frag(q, frag,
-								  offset, len));
-				if (dma_mapping_error(dev, elem->addr))
+				addr = ionic_tx_map_frag(q, frag, offset, len);
+				if (dma_mapping_error(dev, addr))
 					goto err_out_abort;
+				elem->addr = cpu_to_le64(addr);
 				elem->len = cpu_to_le16(len);
 				elem++;
 				desc_nsge++;
-- 
2.17.1

[PATCH net 2/3] ionic: no rx flush in deinit

[Shannon Nelson]

Kmemleak pointed out to us that ionic_rx_flush() is sending
skbs into napi_gro_XXX with a disabled napi context, and these
end up getting lost and leaked.  We can safely remove the flush.

Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c  |  1 -
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 13 -------------
 drivers/net/ethernet/pensando/ionic/ionic_txrx.h |  1 -
 3 files changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 591c644b8e69..a12df3946a07 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1656,7 +1656,6 @@ static void ionic_txrx_deinit(struct ionic_lif *lif)
 	if (lif->rxqcqs) {
 		for (i = 0; i < lif->nxqs && lif->rxqcqs[i]; i++) {
 			ionic_lif_qcq_deinit(lif, lif->rxqcqs[i]);
-			ionic_rx_flush(&lif->rxqcqs[i]->cq);
 			ionic_rx_empty(&lif->rxqcqs[i]->q);
 		}
 	}
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 8f6fc7142bc5..35acb4d66e31 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -253,19 +253,6 @@ static bool ionic_rx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info)
 	return true;
 }
 
-void ionic_rx_flush(struct ionic_cq *cq)
-{
-	struct ionic_dev *idev = &cq->lif->ionic->idev;
-	u32 work_done;
-
-	work_done = ionic_cq_service(cq, cq->num_descs,
-				     ionic_rx_service, NULL, NULL);
-
-	if (work_done)
-		ionic_intr_credits(idev->intr_ctrl, cq->bound_intr->index,
-				   work_done, IONIC_INTR_CRED_RESET_COALESCE);
-}
-
 static int ionic_rx_page_alloc(struct ionic_queue *q,
 			       struct ionic_page_info *page_info)
 {
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.h b/drivers/net/ethernet/pensando/ionic/ionic_txrx.h
index a5883be0413f..7667b72232b8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.h
@@ -4,7 +4,6 @@
 #ifndef _IONIC_TXRX_H_
 #define _IONIC_TXRX_H_
 
-void ionic_rx_flush(struct ionic_cq *cq);
 void ionic_tx_flush(struct ionic_cq *cq);
 
 void ionic_rx_fill(struct ionic_queue *q);
-- 
2.17.1

[PATCH net 3/3] ionic: fix mem leak in rx_empty

[Shannon Nelson]

The sentinel descriptor entry was getting missed in the
traverse of the ring from head to tail, so change to a
loop of 0 to the end.

Fixes: f1d2e894f1b7 ("ionic: use index not pointer for queue tracking")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 35acb4d66e31..b3d2250c77d0 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -400,22 +400,20 @@ static void ionic_rx_fill_cb(void *arg)
 void ionic_rx_empty(struct ionic_queue *q)
 {
 	struct ionic_desc_info *desc_info;
-	struct ionic_rxq_desc *desc;
-	unsigned int i;
-	u16 idx;
-
-	idx = q->tail_idx;
-	while (idx != q->head_idx) {
-		desc_info = &q->info[idx];
-		desc = desc_info->desc;
-		desc->addr = 0;
-		desc->len = 0;
+	struct ionic_page_info *page_info;
+	unsigned int i, j;
 
-		for (i = 0; i < desc_info->npages; i++)
-			ionic_rx_page_free(q, &desc_info->pages[i]);
+	for (i = 0; i < q->num_descs; i++) {
+		desc_info = &q->info[i];
+		for (j = 0; j < IONIC_RX_MAX_SG_ELEMS + 1; j++) {
+			page_info = &desc_info->pages[j];
+			if (page_info->page)
+				ionic_rx_page_free(q, page_info);
+		}
 
+		desc_info->npages = 0;
+		desc_info->cb = NULL;
 		desc_info->cb_arg = NULL;
-		idx = (idx + 1) & (q->num_descs - 1);
 	}
 }
 
-- 
2.17.1

Re: [PATCH net 0/3] ionic: memory usage fixes

[Jakub Kicinski]

On Thu, 22 Oct 2020 16:55:28 -0700 Shannon Nelson wrote:
> This patchset addresses some memory leaks and incorrect
> io reads.

Applied, thanks!

Looks like a fixes tag on patch 3 points to harmless refactoring -
better point to the commit where problem first existed IMO.

Re: [PATCH net 0/3] ionic: memory usage fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Thu, 22 Oct 2020 16:55:28 -0700 you wrote:
> This patchset addresses some memory leaks and incorrect
> io reads.
> 
> Shannon Nelson (3):
>   ionic: clean up sparse complaints
>   ionic: no rx flush in deinit
>   ionic: fix mem leak in rx_empty
> 
> [...]

Here is the summary with links:
  - [net,1/3] ionic: clean up sparse complaints
    https://git.kernel.org/netdev/net/c/d701ec326a31
  - [net,2/3] ionic: no rx flush in deinit
    https://git.kernel.org/netdev/net/c/43ecf7b46f26
  - [net,3/3] ionic: fix mem leak in rx_empty
    https://git.kernel.org/netdev/net/c/0c32a28e247f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html