From: Jakub Kicinski <kuba@kernel.org>
Sent: Thursday, April 15, 2021 10:44 AM
 ...
On Wed, 14 Apr 2021 22:45:19 -0700 Dexuan Cui wrote:

quoted

+	buf = dma_alloc_coherent(gmi->dev, length, &dma_handle,
+				 GFP_KERNEL | __GFP_ZERO);

No need for GFP_ZERO, dma_alloc_coherent() zeroes the memory these days.

Yes, indeed. Will remove __GFP_ZERO.

quoted

+static int mana_gd_register_irq(struct gdma_queue *queue,
+				const struct gdma_queue_spec *spec)
...
+	struct gdma_irq_context *gic;
+
+	struct gdma_context *gc;

Why the empty line?

No good reason. Will remove this line. I'll check the whole patch
for similar issues.

quoted

+	queue = kzalloc(sizeof(*queue), GFP_KERNEL);
+	if (!queue)
+		return -ENOMEM;
+
+	gmi = &queue->mem_info;
+	err = mana_gd_alloc_memory(gc, spec->queue_size, gmi);
+	if (err)
+		return err;

Leaks the memory from 'queue'?

Sorry. This should be a bug I introduced when moving arouond some code.

Same code in mana_gd_create_mana_eq(), ...wq_cq(), etc.

Will fix all of them, and check for the code similar issues.

quoted

+int mana_do_attach(struct net_device *ndev, enum mana_attach_caller

caller)

quoted

+{
+	struct mana_port_context *apc = netdev_priv(ndev);
+	struct gdma_dev *gd = apc->ac->gdma_dev;
+	u32 max_txq, max_rxq, max_queues;
+	int port_idx = apc->port_idx;
+	u32 num_indirect_entries;
+	int err;
+
+	if (caller == MANA_OPEN)
+		goto start_open;
+
+	err = mana_init_port_context(apc);
+	if (err)
+		return err;
+
+	err = mana_query_vport_cfg(apc, port_idx, &max_txq, &max_rxq,
+				   &num_indirect_entries);
+	if (err) {
+		netdev_err(ndev, "Failed to query info for vPort 0\n");
+		goto reset_apc;
+	}
+
+	max_queues = min_t(u32, max_txq, max_rxq);
+	if (apc->max_queues > max_queues)
+		apc->max_queues = max_queues;
+
+	if (apc->num_queues > apc->max_queues)
+		apc->num_queues = apc->max_queues;
+
+	memcpy(ndev->dev_addr, apc->mac_addr, ETH_ALEN);
+
+	if (caller == MANA_PROBE)
+		return 0;
+
+start_open:

Why keep this as a single function, there is no overlap between what's
done for OPEN and PROBE, it seems.

Similarly detach should probably be split into clearly distinct parts.

Will improve the code. Thanks for the suggestion!

Thanks,
Dexuan