On 6/16/2023 11:42 PM, Jakub Kicinski wrote:

On Wed, 14 Jun 2023 10:14:16 -0700 Tony Nguyen wrote:

quoted

+static void idpf_ctlq_init_rxq_bufs(struct idpf_ctlq_info *cq)
+{
+	int i = 0;
+
+	for (i = 0; i < cq->ring_size; i++) {

no need to init i twice

Will fix it.

quoted

+	if (!qinfo->len || !qinfo->buf_size ||
+	    qinfo->len > IDPF_CTLQ_MAX_RING_SIZE ||
+	    qinfo->buf_size > IDPF_CTLQ_MAX_BUF_LEN)
+		return -EINVAL;

Looks like defensive programming, it's generally discouraged in
the kernel.

Thanks for the suggestion, will fix it.

quoted

+init_free_q:
+	kfree(cq);
+	cq = NULL;

no need to clear local variables

Will fix it.

quoted

+	return err;
+}

quoted

+	int i = 0;
+
+	INIT_LIST_HEAD(&hw->cq_list_head);
+
+	for (i = 0; i < num_q; i++) {

init, again, please fix throughout

Sure, will go through all the instances.

quoted

+		struct idpf_ctlq_create_info *qinfo = q_info + i;
+
+		err = idpf_ctlq_add(hw, qinfo, &cq);
+		if (err)
+			goto init_destroy_qs;
+	}
+
+	return err;

return 0 is more idiomatic, you can't reach it with an errno

Will fix it.

quoted

+void idpf_ctlq_deinit(struct idpf_hw *hw)
+{
+	struct idpf_ctlq_info *cq = NULL, *tmp = NULL;

You really like to init the stack :S

Will fix it.

quoted

+	list_for_each_entry_safe(cq, tmp, &hw->cq_list_head, cq_list)
+		idpf_ctlq_remove(hw, cq);
+}

quoted

+	if (!cq || !cq->ring_size)
+		return -ENOBUFS;

even worse defensive programming

Will fix it.

quoted

+	mutex_lock(&cq->cq_lock);
+
+	/* Ensure there are enough descriptors to send all messages */
+	num_desc_avail = IDPF_CTLQ_DESC_UNUSED(cq);
+	if (num_desc_avail == 0 || num_desc_avail < num_q_msg) {
+		err = -ENOSPC;
+		goto sq_send_command_out;

name labels after what they jump to, err_unlock

Will fix it.

quoted

+void *idpf_alloc_dma_mem(struct idpf_hw *hw, struct idpf_dma_mem *mem, u64 size)
+{
+	struct idpf_adapter *adapter = hw->back;
+	size_t sz = ALIGN(size, 4096);
+
+	mem->va = dma_alloc_coherent(&adapter->pdev->dev, sz,
+				     &mem->pa, GFP_KERNEL | __GFP_ZERO);

DMA API always zeros memory, I thought cocci warns about this
Did you run cocci checks?

I ran cocci check using the command "make -j8 
M=drivers/net/ethernet/intel/idpf/ C=1 CHECK="scripts/coccicheck" 
CONFIG_IDPF=m &>>err.log" but didn't see any hits and not sure if this 
is the right command to see the warning. Will fix it anyways.

Thanks,
Pavan