[PATCH 0/3] megaraid_sas: copule of fixes

[PATCH 0/3] megaraid_sas: copule of fixes

[Weidong Wang]

Fix some checkpatch Warnings.
Fix a NULL pointer dereference.

As well kernel >=3.0.y will have the 'NULL pointer dereference'.

Weidong Wang (3):
  megaraid_sas: Convert dev_printk to dev_<level>
  megaraid_sas: Convert printk to printk_<level>
  megaraid_sas: return -ENOMEM when create DMA pool for cmd frames
    failed

 drivers/scsi/megaraid/megaraid_sas_base.c | 83 ++++++++++++++++---------------
 1 file changed, 42 insertions(+), 41 deletions(-)

-- 
1.9.0

[PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Weidong Wang]

Reduce object size a little by using dev_<level>
calls instead of dev_printk(KERN_<LEVEL>.

Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 68 +++++++++++++++----------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index eaa81e5..ed9846d 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1884,7 +1884,7 @@ static int megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
+		dev_dbg(&instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
 		       "Failed to get cmd for scsi%d\n",
 			instance->host->host_no);
 		return -ENOMEM;
@@ -1908,7 +1908,7 @@ static int megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
 					     sizeof(struct MR_LD_VF_AFFILIATION_111),
 					     &new_affiliation_111_h);
 		if (!new_affiliation_111) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV: Couldn't allocate "
+			dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't allocate "
 			       "memory for new affiliation for scsi%d\n",
 			       instance->host->host_no);
 			megasas_return_cmd(instance, cmd);
@@ -1995,7 +1995,7 @@ static int megasas_get_ld_vf_affiliation_12(struct megasas_instance *instance,
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_get_ld_vf_affiliation12: "
+		dev_dbg(&instance->pdev->dev, "megasas_get_ld_vf_affiliation12: "
 		       "Failed to get cmd for scsi%d\n",
 		       instance->host->host_no);
 		return -ENOMEM;
@@ -2020,7 +2020,7 @@ static int megasas_get_ld_vf_affiliation_12(struct megasas_instance *instance,
 					     sizeof(struct MR_LD_VF_AFFILIATION),
 					     &new_affiliation_h);
 		if (!new_affiliation) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV: Couldn't allocate "
+			dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't allocate "
 			       "memory for new affiliation for scsi%d\n",
 			       instance->host->host_no);
 			megasas_return_cmd(instance, cmd);
@@ -2174,7 +2174,7 @@ int megasas_sriov_start_heartbeat(struct megasas_instance *instance,
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_sriov_start_heartbeat: "
+		dev_dbg(&instance->pdev->dev, "megasas_sriov_start_heartbeat: "
 		       "Failed to get cmd for scsi%d\n",
 		       instance->host->host_no);
 		return -ENOMEM;
@@ -2188,7 +2188,7 @@ int megasas_sriov_start_heartbeat(struct megasas_instance *instance,
 					      sizeof(struct MR_CTRL_HB_HOST_MEM),
 					      &instance->hb_host_mem_h);
 		if (!instance->hb_host_mem) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV: Couldn't allocate"
+			dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't allocate"
 			       " memory for heartbeat host memory for scsi%d\n",
 			       instance->host->host_no);
 			retval = -ENOMEM;
@@ -2922,7 +2922,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 			break;
 
 		default:
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "MFI FW status %#x\n",
+			dev_dbg(&instance->pdev->dev, "MFI FW status %#x\n",
 			       hdr->cmd_status);
 			cmd->scmd->result = DID_ERROR << 16;
 			break;
@@ -3332,7 +3332,7 @@ megasas_transition_to_ready(struct megasas_instance *instance, int ocr)
 		switch (fw_state) {
 
 		case MFI_STATE_FAULT:
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "FW in FAULT state!!\n");
+			dev_dbg(&instance->pdev->dev, "FW in FAULT state!!\n");
 			if (ocr) {
 				max_wait = MEGASAS_RESET_WAIT_TIME;
 				cur_state = MFI_STATE_FAULT;
@@ -3471,7 +3471,7 @@ megasas_transition_to_ready(struct megasas_instance *instance, int ocr)
 			break;
 
 		default:
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "Unknown state 0x%x\n",
+			dev_dbg(&instance->pdev->dev, "Unknown state 0x%x\n",
 			       fw_state);
 			return -ENODEV;
 		}
@@ -3493,7 +3493,7 @@ megasas_transition_to_ready(struct megasas_instance *instance, int ocr)
 		 * Return error if fw_state hasn't changed after max_wait
 		 */
 		if (curr_abs_state == abs_state) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "FW state [%d] hasn't changed "
+			dev_dbg(&instance->pdev->dev, "FW state [%d] hasn't changed "
 			       "in %d secs\n", fw_state, max_wait);
 			return -ENODEV;
 		}
@@ -3595,7 +3595,7 @@ static int megasas_create_frame_pool(struct megasas_instance *instance)
 					instance->pdev, total_sz, 256, 0);
 
 	if (!instance->frame_dma_pool) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "failed to setup frame pool\n");
+		dev_dbg(&instance->pdev->dev, "failed to setup frame pool\n");
 		return -ENOMEM;
 	}
 
@@ -3603,7 +3603,7 @@ static int megasas_create_frame_pool(struct megasas_instance *instance)
 						   instance->pdev, 128, 4, 0);
 
 	if (!instance->sense_dma_pool) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "failed to setup sense pool\n");
+		dev_dbg(&instance->pdev->dev, "failed to setup sense pool\n");
 
 		pci_pool_destroy(instance->frame_dma_pool);
 		instance->frame_dma_pool = NULL;
@@ -3631,7 +3631,7 @@ static int megasas_create_frame_pool(struct megasas_instance *instance)
 		 * whatever has been allocated
 		 */
 		if (!cmd->frame || !cmd->sense) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "pci_pool_alloc failed\n");
+			dev_dbg(&instance->pdev->dev, "pci_pool_alloc failed\n");
 			megasas_teardown_frame_pool(instance);
 			return -ENOMEM;
 		}
@@ -3710,7 +3710,7 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
 	instance->cmd_list = kcalloc(max_cmd, sizeof(struct megasas_cmd*), GFP_KERNEL);
 
 	if (!instance->cmd_list) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "out of memory\n");
+		dev_dbg(&instance->pdev->dev, "out of memory\n");
 		return -ENOMEM;
 	}
 
@@ -3746,7 +3746,7 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
 	 * Create a frame pool and assign one frame to each cmd
 	 */
 	if (megasas_create_frame_pool(instance)) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Error creating frame DMA pool\n");
+		dev_dbg(&instance->pdev->dev, "Error creating frame DMA pool\n");
 		megasas_free_cmds(instance);
 	}
 
@@ -3775,7 +3775,7 @@ megasas_get_pd_list(struct megasas_instance *instance)
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "(get_pd_list): Failed to get cmd\n");
+		dev_dbg(&instance->pdev->dev, "(get_pd_list): Failed to get cmd\n");
 		return -ENOMEM;
 	}
 
@@ -3785,7 +3785,7 @@ megasas_get_pd_list(struct megasas_instance *instance)
 		  MEGASAS_MAX_PD * sizeof(struct MR_PD_LIST), &ci_h);
 
 	if (!ci) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to alloc mem for pd_list\n");
+		dev_dbg(&instance->pdev->dev, "Failed to alloc mem for pd_list\n");
 		megasas_return_cmd(instance, cmd);
 		return -ENOMEM;
 	}
@@ -3870,7 +3870,7 @@ megasas_get_ld_list(struct megasas_instance *instance)
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_get_ld_list: Failed to get cmd\n");
+		dev_dbg(&instance->pdev->dev, "megasas_get_ld_list: Failed to get cmd\n");
 		return -ENOMEM;
 	}
 
@@ -3881,7 +3881,7 @@ megasas_get_ld_list(struct megasas_instance *instance)
 				&ci_h);
 
 	if (!ci) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to alloc mem in get_ld_list\n");
+		dev_dbg(&instance->pdev->dev, "Failed to alloc mem in get_ld_list\n");
 		megasas_return_cmd(instance, cmd);
 		return -ENOMEM;
 	}
@@ -4094,7 +4094,7 @@ megasas_get_ctrl_info(struct megasas_instance *instance)
 	cmd = megasas_get_cmd(instance);
 
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to get a free cmd\n");
+		dev_dbg(&instance->pdev->dev, "Failed to get a free cmd\n");
 		return -ENOMEM;
 	}
 
@@ -4104,7 +4104,7 @@ megasas_get_ctrl_info(struct megasas_instance *instance)
 				  sizeof(struct megasas_ctrl_info), &ci_h);
 
 	if (!ci) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to alloc mem for ctrl info\n");
+		dev_dbg(&instance->pdev->dev, "Failed to alloc mem for ctrl info\n");
 		megasas_return_cmd(instance, cmd);
 		return -ENOMEM;
 	}
@@ -4341,7 +4341,7 @@ megasas_init_adapter_mfi(struct megasas_instance *instance)
 						     &instance->reply_queue_h);
 
 	if (!instance->reply_queue) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Out of DMA mem for reply queue\n");
+		dev_dbg(&instance->pdev->dev, "Out of DMA mem for reply queue\n");
 		goto fail_reply_queue;
 	}
 
@@ -4504,7 +4504,7 @@ static int megasas_init_fw(struct megasas_instance *instance)
 	instance->bar = find_first_bit(&bar_list, sizeof(unsigned long));
 	if (pci_request_selected_regions(instance->pdev, instance->bar,
 					 "megasas: LSI")) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "IO memory region busy!\n");
+		dev_dbg(&instance->pdev->dev, "IO memory region busy!\n");
 		return -EBUSY;
 	}
 
@@ -4512,7 +4512,7 @@ static int megasas_init_fw(struct megasas_instance *instance)
 	instance->reg_set = ioremap_nocache(base_addr, 8192);
 
 	if (!instance->reg_set) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to map IO mem\n");
+		dev_dbg(&instance->pdev->dev, "Failed to map IO mem\n");
 		goto fail_ioremap;
 	}
 
@@ -4958,7 +4958,7 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num,
 								  aen_cmd, 30);
 
 			if (ret_val) {
-				dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to abort "
+				dev_dbg(&instance->pdev->dev, "Failed to abort "
 				       "previous AEN command\n");
 				return ret_val;
 			}
@@ -5204,7 +5204,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
 			       sizeof(struct megasas_instance));
 
 	if (!host) {
-		dev_printk(KERN_DEBUG, &pdev->dev, "scsi_host_alloc failed\n");
+		dev_dbg(&pdev->dev, "scsi_host_alloc failed\n");
 		goto fail_alloc_instance;
 	}
 
@@ -5224,7 +5224,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
 		instance->ctrl_context = (void *)__get_free_pages(GFP_KERNEL,
 				instance->ctrl_context_pages);
 		if (!instance->ctrl_context) {
-			dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate "
+			dev_dbg(&pdev->dev, "Failed to allocate "
 			       "memory for Fusion context info\n");
 			goto fail_alloc_dma_buf;
 		}
@@ -5243,7 +5243,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
 					     &instance->consumer_h);
 
 		if (!instance->producer || !instance->consumer) {
-			dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate"
+			dev_dbg(&pdev->dev, "Failed to allocate"
 			       "memory for producer, consumer\n");
 			goto fail_alloc_dma_buf;
 		}
@@ -5290,7 +5290,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
 						    &instance->evt_detail_h);
 
 	if (!instance->evt_detail) {
-		dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate memory for "
+		dev_dbg(&pdev->dev, "Failed to allocate memory for "
 		       "event detail structure\n");
 		goto fail_alloc_dma_buf;
 	}
@@ -5397,7 +5397,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
 	 * Initiate AEN (Asynchronous Event Notification)
 	 */
 	if (megasas_start_aen(instance)) {
-		dev_printk(KERN_DEBUG, &pdev->dev, "start aen failed\n");
+		dev_dbg(&pdev->dev, "start aen failed\n");
 		goto fail_start_aen;
 	}
 
@@ -5973,14 +5973,14 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 	memset(kbuff_arr, 0, sizeof(kbuff_arr));
 
 	if (ioc->sge_count > MAX_IOCTL_SGE) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "SGE count [%d] >  max limit [%d]\n",
+		dev_dbg(&instance->pdev->dev, "SGE count [%d] >  max limit [%d]\n",
 		       ioc->sge_count, MAX_IOCTL_SGE);
 		return -EINVAL;
 	}
 
 	cmd = megasas_get_cmd(instance);
 	if (!cmd) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to get a cmd packet\n");
+		dev_dbg(&instance->pdev->dev, "Failed to get a cmd packet\n");
 		return -ENOMEM;
 	}
 
@@ -6025,7 +6025,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 						    ioc->sgl[i].iov_len,
 						    &buf_handle, GFP_KERNEL);
 		if (!kbuff_arr[i]) {
-			dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed to alloc "
+			dev_dbg(&instance->pdev->dev, "Failed to alloc "
 			       "kernel SGL buffer for IOCTL\n");
 			error = -ENOMEM;
 			goto out;
@@ -6111,7 +6111,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 	 */
 	if (copy_to_user(&user_ioc->frame.hdr.cmd_status,
 			 &cmd->frame->hdr.cmd_status, sizeof(u8))) {
-		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Error copying out cmd_status\n");
+		dev_dbg(&instance->pdev->dev, "Error copying out cmd_status\n");
 		error = -EFAULT;
 	}
 
-- 
1.9.0

[PATCH 2/3] megaraid_sas: Convert printk to printk_<level>

[Weidong Wang]

Reduce object size a little by using pr_<level>
calls instead of printk(KERN_<LEVEL>.

Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index ed9846d..2287aa1 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5889,7 +5889,7 @@ static int megasas_mgmt_fasync(int fd, struct file *filep, int mode)
 		return 0;
 	}
 
-	printk(KERN_DEBUG "megasas: fasync_helper failed [%d]\n", rc);
+	pr_debug("megasas: fasync_helper failed [%d]\n", rc);
 
 	return rc;
 }
@@ -6233,7 +6233,7 @@ static int megasas_mgmt_ioctl_aen(struct file *file, unsigned long arg)
 	u32 wait_time = MEGASAS_RESET_WAIT_TIME;
 
 	if (file->private_data != file) {
-		printk(KERN_DEBUG "megasas: fasync_helper was not "
+		pr_debug("megasas: fasync_helper was not "
 		       "called first\n");
 		return -EINVAL;
 	}
@@ -6355,7 +6355,7 @@ static int megasas_mgmt_compat_ioctl_fw(struct file *file, unsigned long arg)
 
 	if (copy_in_user(&cioc->frame.hdr.cmd_status,
 			 &ioc->frame.hdr.cmd_status, sizeof(u8))) {
-		printk(KERN_DEBUG "megasas: error copy_in_user cmd_status\n");
+		pr_debug("megasas: error copy_in_user cmd_status\n");
 		return -EFAULT;
 	}
 	return error;
@@ -6455,7 +6455,7 @@ megasas_sysfs_set_dbg_lvl(struct device_driver *dd, const char *buf, size_t coun
 	int retval = count;
 
 	if (sscanf(buf, "%u", &megasas_dbg_lvl) < 1) {
-		printk(KERN_ERR "megasas: could not set dbg_lvl\n");
+		pr_err("megasas: could not set dbg_lvl\n");
 		retval = -EINVAL;
 	}
 	return retval;
@@ -6480,7 +6480,7 @@ megasas_aen_polling(struct work_struct *work)
 	int error;
 
 	if (!instance) {
-		printk(KERN_ERR "invalid instance!\n");
+		pr_err("invalid instance!\n");
 		kfree(ev);
 		return;
 	}
@@ -6740,7 +6740,7 @@ static int __init megasas_init(void)
 	rval = register_chrdev(0, "megaraid_sas_ioctl", &megasas_mgmt_fops);
 
 	if (rval < 0) {
-		printk(KERN_DEBUG "megasas: failed to open device node\n");
+		pr_debug("megasas: failed to open device node\n");
 		return rval;
 	}
 
@@ -6752,7 +6752,7 @@ static int __init megasas_init(void)
 	rval = pci_register_driver(&megasas_pci_driver);
 
 	if (rval) {
-		printk(KERN_DEBUG "megasas: PCI hotplug registration failed \n");
+		pr_debug("megasas: PCI hotplug registration failed \n");
 		goto err_pcidrv;
 	}
 
-- 
1.9.0

[PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed

[Weidong Wang]

when create DMA pool for cmd frames failed, we should return -ENOMEM,
instead of 0.
In some case in:

    megasas_init_adapter_fusion()

    -->megasas_alloc_cmds()
       -->megasas_create_frame_pool
          create DMA pool failed,
        --> megasas_free_cmds() [1]

    -->megasas_alloc_cmds_fusion()
       failed, then goto fail_alloc_cmds.
    -->megasas_free_cmds() [2]

we will call megasas_free_cmds twice, [1] will kfree cmd_list,
[2] will use cmd_list.it will cause a problem:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = ffffffc000f70000
[00000000] *pgd=0000001fbf893003, *pud=0000001fbf893003, *pmd=0000001fbf894003, *pte=006000006d000707
Internal error: Oops: 96000005 [#1] SMP
 Modules linked in:
 CPU: 18 PID: 1 Comm: swapper/0 Not tainted
 task: ffffffdfb9290000 ti: ffffffdfb923c000 task.ti: ffffffdfb923c000
 PC is at megasas_free_cmds+0x30/0x70
 LR is at megasas_free_cmds+0x24/0x70

 ...

 Call trace:
 [<ffffffc0005b779c>] megasas_free_cmds+0x30/0x70
 [<ffffffc0005bca74>] megasas_init_adapter_fusion+0x2f4/0x4d8
 [<ffffffc0005b926c>] megasas_init_fw+0x2dc/0x760
 [<ffffffc0005b9ab0>] megasas_probe_one+0x3c0/0xcd8
 [<ffffffc0004a5abc>] local_pci_probe+0x4c/0xb4
 [<ffffffc0004a5c40>] pci_device_probe+0x11c/0x14c
 [<ffffffc00053a5e4>] driver_probe_device+0x1ec/0x430
 [<ffffffc00053a92c>] __driver_attach+0xa8/0xb0
 [<ffffffc000538178>] bus_for_each_dev+0x74/0xc8
  [<ffffffc000539e88>] driver_attach+0x28/0x34
 [<ffffffc000539a18>] bus_add_driver+0x16c/0x248
 [<ffffffc00053b234>] driver_register+0x6c/0x138
 [<ffffffc0004a5350>] __pci_register_driver+0x5c/0x6c
 [<ffffffc000ce3868>] megasas_init+0xc0/0x1a8
 [<ffffffc000082a58>] do_one_initcall+0xe8/0x1ec
 [<ffffffc000ca7be8>] kernel_init_freeable+0x1c8/0x284
 [<ffffffc0008d90b8>] kernel_init+0x1c/0xe4

Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 2287aa1..8215218 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3746,8 +3746,9 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
 	 * Create a frame pool and assign one frame to each cmd
 	 */
 	if (megasas_create_frame_pool(instance)) {
-		dev_dbg(&instance->pdev->dev, "Error creating frame DMA pool\n");
+		dev_err(&instance->pdev->dev, "Error creating frame DMA pool\n");
 		megasas_free_cmds(instance);
+		return -ENOMEM;
 	}
 
 	return 0;
-- 
1.9.0

Re: [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed

[Johannes Thumshirn]

On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
> when create DMA pool for cmd frames failed, we should return -ENOMEM,
> instead of 0.
> In some case in:
> 
>     megasas_init_adapter_fusion()
> 
>     -->megasas_alloc_cmds()
>        -->megasas_create_frame_pool
>           create DMA pool failed,
>         --> megasas_free_cmds() [1]
> 
>     -->megasas_alloc_cmds_fusion()
>        failed, then goto fail_alloc_cmds.
>     -->megasas_free_cmds() [2]
> 
> we will call megasas_free_cmds twice, [1] will kfree cmd_list,
> [2] will use cmd_list.it will cause a problem:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000
> pgd = ffffffc000f70000
> [00000000] *pgd=0000001fbf893003, *pud=0000001fbf893003,
> *pmd=0000001fbf894003, *pte=006000006d000707
> Internal error: Oops: 96000005 [#1] SMP
>  Modules linked in:
>  CPU: 18 PID: 1 Comm: swapper/0 Not tainted
>  task: ffffffdfb9290000 ti: ffffffdfb923c000 task.ti:
> ffffffdfb923c000
>  PC is at megasas_free_cmds+0x30/0x70
>  LR is at megasas_free_cmds+0x24/0x70
> 
>  ...
> 
>  Call trace:
>  [<ffffffc0005b779c>] megasas_free_cmds+0x30/0x70
>  [<ffffffc0005bca74>] megasas_init_adapter_fusion+0x2f4/0x4d8
>  [<ffffffc0005b926c>] megasas_init_fw+0x2dc/0x760
>  [<ffffffc0005b9ab0>] megasas_probe_one+0x3c0/0xcd8
>  [<ffffffc0004a5abc>] local_pci_probe+0x4c/0xb4
>  [<ffffffc0004a5c40>] pci_device_probe+0x11c/0x14c
>  [<ffffffc00053a5e4>] driver_probe_device+0x1ec/0x430
>  [<ffffffc00053a92c>] __driver_attach+0xa8/0xb0
>  [<ffffffc000538178>] bus_for_each_dev+0x74/0xc8
>   [<ffffffc000539e88>] driver_attach+0x28/0x34
>  [<ffffffc000539a18>] bus_add_driver+0x16c/0x248
>  [<ffffffc00053b234>] driver_register+0x6c/0x138
>  [<ffffffc0004a5350>] __pci_register_driver+0x5c/0x6c
>  [<ffffffc000ce3868>] megasas_init+0xc0/0x1a8
>  [<ffffffc000082a58>] do_one_initcall+0xe8/0x1ec
>  [<ffffffc000ca7be8>] kernel_init_freeable+0x1c8/0x284
>  [<ffffffc0008d90b8>] kernel_init+0x1c/0xe4
> 
> Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 2287aa1..8215218 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -3746,8 +3746,9 @@ int megasas_alloc_cmds(struct megasas_instance
> *instance)
>  	 * Create a frame pool and assign one frame to each cmd
>  	 */
>  	if (megasas_create_frame_pool(instance)) {
> -		dev_dbg(&instance->pdev->dev, "Error creating frame
> DMA pool\n");
> +		dev_err(&instance->pdev->dev, "Error creating frame
> DMA pool\n");
>  		megasas_free_cmds(instance);
> +		return -ENOMEM;
>  	}
>  
>  	return 0;

I think this is needed for stable as well.

Other than that,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Thanks,
	Johannes

Re: [PATCH 2/3] megaraid_sas: Convert printk to printk_<level>

[Johannes Thumshirn]

On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
> Reduce object size a little by using pr_<level>
> calls instead of printk(KERN_<LEVEL>.
> 
> Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index ed9846d..2287aa1 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -5889,7 +5889,7 @@ static int megasas_mgmt_fasync(int fd, struct
> file *filep, int mode)
>  		return 0;
>  	}
>  
> -	printk(KERN_DEBUG "megasas: fasync_helper failed [%d]\n",
> rc);
> +	pr_debug("megasas: fasync_helper failed [%d]\n", rc);
>  
>  	return rc;
>  }
> @@ -6233,7 +6233,7 @@ static int megasas_mgmt_ioctl_aen(struct file
> *file, unsigned long arg)
>  	u32 wait_time = MEGASAS_RESET_WAIT_TIME;
>  
>  	if (file->private_data != file) {
> -		printk(KERN_DEBUG "megasas: fasync_helper was not "
> +		pr_debug("megasas: fasync_helper was not "
>  		       "called first\n");
>  		return -EINVAL;
>  	}
> @@ -6355,7 +6355,7 @@ static int megasas_mgmt_compat_ioctl_fw(struct
> file *file, unsigned long arg)
>  
>  	if (copy_in_user(&cioc->frame.hdr.cmd_status,
>  			 &ioc->frame.hdr.cmd_status, sizeof(u8))) {
> -		printk(KERN_DEBUG "megasas: error copy_in_user
> cmd_status\n");
> +		pr_debug("megasas: error copy_in_user
> cmd_status\n");
>  		return -EFAULT;
>  	}
>  	return error;
> @@ -6455,7 +6455,7 @@ megasas_sysfs_set_dbg_lvl(struct device_driver
> *dd, const char *buf, size_t coun
>  	int retval = count;
>  
>  	if (sscanf(buf, "%u", &megasas_dbg_lvl) < 1) {
> -		printk(KERN_ERR "megasas: could not set dbg_lvl\n");
> +		pr_err("megasas: could not set dbg_lvl\n");
>  		retval = -EINVAL;
>  	}
>  	return retval;
> @@ -6480,7 +6480,7 @@ megasas_aen_polling(struct work_struct *work)
>  	int error;
>  
>  	if (!instance) {
> -		printk(KERN_ERR "invalid instance!\n");
> +		pr_err("invalid instance!\n");
>  		kfree(ev);
>  		return;
>  	}
> @@ -6740,7 +6740,7 @@ static int __init megasas_init(void)
>  	rval = register_chrdev(0, "megaraid_sas_ioctl",
> &megasas_mgmt_fops);
>  
>  	if (rval < 0) {
> -		printk(KERN_DEBUG "megasas: failed to open device
> node\n");
> +		pr_debug("megasas: failed to open device node\n");
>  		return rval;
>  	}
>  
> @@ -6752,7 +6752,7 @@ static int __init megasas_init(void)
>  	rval = pci_register_driver(&megasas_pci_driver);
>  
>  	if (rval) {
> -		printk(KERN_DEBUG "megasas: PCI hotplug registration
> failed \n");
> +		pr_debug("megasas: PCI hotplug registration failed
> \n");
>  		goto err_pcidrv;
>  	}
>  


Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Johannes Thumshirn]

On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
> Reduce object size a little by using dev_<level>
> calls instead of dev_printk(KERN_<LEVEL>.
> 
> Signed-off-by: Weidong Wang <wangweidong1@huawei.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 68 +++++++++++++++----
> ------------
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index eaa81e5..ed9846d 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1884,7 +1884,7 @@ static int
> megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev,
> "megasas_get_ld_vf_affiliation_111:"
> +		dev_dbg(&instance->pdev->dev,
> "megasas_get_ld_vf_affiliation_111:"
>  		       "Failed to get cmd for scsi%d\n",
>  			instance->host->host_no);
>  		return -ENOMEM;
> @@ -1908,7 +1908,7 @@ static int
> megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
>  					     sizeof(struct
> MR_LD_VF_AFFILIATION_111),
>  					     &new_affiliation_111_h)
> ;
>  		if (!new_affiliation_111) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "SR-IOV: Couldn't allocate "
> +			dev_dbg(&instance->pdev->dev, "SR-IOV:
> Couldn't allocate "
>  			       "memory for new affiliation for
> scsi%d\n",
>  			       instance->host->host_no);
>  			megasas_return_cmd(instance, cmd);
> @@ -1995,7 +1995,7 @@ static int
> megasas_get_ld_vf_affiliation_12(struct megasas_instance *instance,
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev,
> "megasas_get_ld_vf_affiliation12: "
> +		dev_dbg(&instance->pdev->dev,
> "megasas_get_ld_vf_affiliation12: "
>  		       "Failed to get cmd for scsi%d\n",
>  		       instance->host->host_no);
>  		return -ENOMEM;
> @@ -2020,7 +2020,7 @@ static int
> megasas_get_ld_vf_affiliation_12(struct megasas_instance *instance,
>  					     sizeof(struct
> MR_LD_VF_AFFILIATION),
>  					     &new_affiliation_h);
>  		if (!new_affiliation) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "SR-IOV: Couldn't allocate "
> +			dev_dbg(&instance->pdev->dev, "SR-IOV:
> Couldn't allocate "
>  			       "memory for new affiliation for
> scsi%d\n",
>  			       instance->host->host_no);
>  			megasas_return_cmd(instance, cmd);
> @@ -2174,7 +2174,7 @@ int megasas_sriov_start_heartbeat(struct
> megasas_instance *instance,
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev,
> "megasas_sriov_start_heartbeat: "
> +		dev_dbg(&instance->pdev->dev,
> "megasas_sriov_start_heartbeat: "
>  		       "Failed to get cmd for scsi%d\n",
>  		       instance->host->host_no);
>  		return -ENOMEM;
> @@ -2188,7 +2188,7 @@ int megasas_sriov_start_heartbeat(struct
> megasas_instance *instance,
>  					      sizeof(struct
> MR_CTRL_HB_HOST_MEM),
>  					      &instance-
> >hb_host_mem_h);
>  		if (!instance->hb_host_mem) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "SR-IOV: Couldn't allocate"
> +			dev_dbg(&instance->pdev->dev, "SR-IOV:
> Couldn't allocate"
>  			       " memory for heartbeat host memory
> for scsi%d\n",
>  			       instance->host->host_no);
>  			retval = -ENOMEM;
> @@ -2922,7 +2922,7 @@ megasas_complete_cmd(struct megasas_instance
> *instance, struct megasas_cmd *cmd,
>  			break;
>  
>  		default:
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "MFI FW status %#x\n",
> +			dev_dbg(&instance->pdev->dev, "MFI FW status
> %#x\n",
>  			       hdr->cmd_status);
>  			cmd->scmd->result = DID_ERROR << 16;
>  			break;
> @@ -3332,7 +3332,7 @@ megasas_transition_to_ready(struct
> megasas_instance *instance, int ocr)
>  		switch (fw_state) {
>  
>  		case MFI_STATE_FAULT:
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "FW in FAULT state!!\n");
> +			dev_dbg(&instance->pdev->dev, "FW in FAULT
> state!!\n");
>  			if (ocr) {
>  				max_wait = MEGASAS_RESET_WAIT_TIME;
>  				cur_state = MFI_STATE_FAULT;
> @@ -3471,7 +3471,7 @@ megasas_transition_to_ready(struct
> megasas_instance *instance, int ocr)
>  			break;
>  
>  		default:
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "Unknown state 0x%x\n",
> +			dev_dbg(&instance->pdev->dev, "Unknown state
> 0x%x\n",
>  			       fw_state);
>  			return -ENODEV;
>  		}
> @@ -3493,7 +3493,7 @@ megasas_transition_to_ready(struct
> megasas_instance *instance, int ocr)
>  		 * Return error if fw_state hasn't changed after
> max_wait
>  		 */
>  		if (curr_abs_state == abs_state) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "FW state [%d] hasn't changed "
> +			dev_dbg(&instance->pdev->dev, "FW state [%d]
> hasn't changed "
>  			       "in %d secs\n", fw_state, max_wait);
>  			return -ENODEV;
>  		}
> @@ -3595,7 +3595,7 @@ static int megasas_create_frame_pool(struct
> megasas_instance *instance)
>  					instance->pdev, total_sz,
> 256, 0);
>  
>  	if (!instance->frame_dma_pool) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "failed
> to setup frame pool\n");
> +		dev_dbg(&instance->pdev->dev, "failed to setup frame
> pool\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -3603,7 +3603,7 @@ static int megasas_create_frame_pool(struct
> megasas_instance *instance)
>  						   instance->pdev,
> 128, 4, 0);
>  
>  	if (!instance->sense_dma_pool) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "failed
> to setup sense pool\n");
> +		dev_dbg(&instance->pdev->dev, "failed to setup sense
> pool\n");
>  
>  		pci_pool_destroy(instance->frame_dma_pool);
>  		instance->frame_dma_pool = NULL;
> @@ -3631,7 +3631,7 @@ static int megasas_create_frame_pool(struct
> megasas_instance *instance)
>  		 * whatever has been allocated
>  		 */
>  		if (!cmd->frame || !cmd->sense) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "pci_pool_alloc failed\n");
> +			dev_dbg(&instance->pdev->dev,
> "pci_pool_alloc failed\n");
>  			megasas_teardown_frame_pool(instance);
>  			return -ENOMEM;
>  		}
> @@ -3710,7 +3710,7 @@ int megasas_alloc_cmds(struct megasas_instance
> *instance)
>  	instance->cmd_list = kcalloc(max_cmd, sizeof(struct
> megasas_cmd*), GFP_KERNEL);
>  
>  	if (!instance->cmd_list) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "out of
> memory\n");
> +		dev_dbg(&instance->pdev->dev, "out of memory\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -3746,7 +3746,7 @@ int megasas_alloc_cmds(struct megasas_instance
> *instance)
>  	 * Create a frame pool and assign one frame to each cmd
>  	 */
>  	if (megasas_create_frame_pool(instance)) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Error
> creating frame DMA pool\n");
> +		dev_dbg(&instance->pdev->dev, "Error creating frame
> DMA pool\n");
>  		megasas_free_cmds(instance);
>  	}
>  
> @@ -3775,7 +3775,7 @@ megasas_get_pd_list(struct megasas_instance
> *instance)
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev,
> "(get_pd_list): Failed to get cmd\n");
> +		dev_dbg(&instance->pdev->dev, "(get_pd_list): Failed
> to get cmd\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -3785,7 +3785,7 @@ megasas_get_pd_list(struct megasas_instance
> *instance)
>  		  MEGASAS_MAX_PD * sizeof(struct MR_PD_LIST),
> &ci_h);
>  
>  	if (!ci) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to alloc mem for pd_list\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to alloc mem
> for pd_list\n");
>  		megasas_return_cmd(instance, cmd);
>  		return -ENOMEM;
>  	}
> @@ -3870,7 +3870,7 @@ megasas_get_ld_list(struct megasas_instance
> *instance)
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev,
> "megasas_get_ld_list: Failed to get cmd\n");
> +		dev_dbg(&instance->pdev->dev, "megasas_get_ld_list:
> Failed to get cmd\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -3881,7 +3881,7 @@ megasas_get_ld_list(struct megasas_instance
> *instance)
>  				&ci_h);
>  
>  	if (!ci) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to alloc mem in get_ld_list\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to alloc mem
> in get_ld_list\n");
>  		megasas_return_cmd(instance, cmd);
>  		return -ENOMEM;
>  	}
> @@ -4094,7 +4094,7 @@ megasas_get_ctrl_info(struct megasas_instance
> *instance)
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to get a free cmd\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to get a free
> cmd\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -4104,7 +4104,7 @@ megasas_get_ctrl_info(struct megasas_instance
> *instance)
>  				  sizeof(struct megasas_ctrl_info),
> &ci_h);
>  
>  	if (!ci) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to alloc mem for ctrl info\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to alloc mem
> for ctrl info\n");
>  		megasas_return_cmd(instance, cmd);
>  		return -ENOMEM;
>  	}
> @@ -4341,7 +4341,7 @@ megasas_init_adapter_mfi(struct
> megasas_instance *instance)
>  						     &instance-
> >reply_queue_h);
>  
>  	if (!instance->reply_queue) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Out of
> DMA mem for reply queue\n");
> +		dev_dbg(&instance->pdev->dev, "Out of DMA mem for
> reply queue\n");
>  		goto fail_reply_queue;
>  	}
>  
> @@ -4504,7 +4504,7 @@ static int megasas_init_fw(struct
> megasas_instance *instance)
>  	instance->bar = find_first_bit(&bar_list, sizeof(unsigned
> long));
>  	if (pci_request_selected_regions(instance->pdev, instance-
> >bar,
>  					 "megasas: LSI")) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "IO
> memory region busy!\n");
> +		dev_dbg(&instance->pdev->dev, "IO memory region
> busy!\n");
>  		return -EBUSY;
>  	}
>  
> @@ -4512,7 +4512,7 @@ static int megasas_init_fw(struct
> megasas_instance *instance)
>  	instance->reg_set = ioremap_nocache(base_addr, 8192);
>  
>  	if (!instance->reg_set) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to map IO mem\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to map IO
> mem\n");
>  		goto fail_ioremap;
>  	}
>  
> @@ -4958,7 +4958,7 @@ megasas_register_aen(struct megasas_instance
> *instance, u32 seq_num,
>  								  ae
> n_cmd, 30);
>  
>  			if (ret_val) {
> -				dev_printk(KERN_DEBUG, &instance-
> >pdev->dev, "Failed to abort "
> +				dev_dbg(&instance->pdev->dev,
> "Failed to abort "
>  				       "previous AEN command\n");
>  				return ret_val;
>  			}
> @@ -5204,7 +5204,7 @@ static int megasas_probe_one(struct pci_dev
> *pdev,
>  			       sizeof(struct megasas_instance));
>  
>  	if (!host) {
> -		dev_printk(KERN_DEBUG, &pdev->dev, "scsi_host_alloc
> failed\n");
> +		dev_dbg(&pdev->dev, "scsi_host_alloc failed\n");
>  		goto fail_alloc_instance;
>  	}
>  
> @@ -5224,7 +5224,7 @@ static int megasas_probe_one(struct pci_dev
> *pdev,
>  		instance->ctrl_context = (void
> *)__get_free_pages(GFP_KERNEL,
>  				instance->ctrl_context_pages);
>  		if (!instance->ctrl_context) {
> -			dev_printk(KERN_DEBUG, &pdev->dev, "Failed
> to allocate "
> +			dev_dbg(&pdev->dev, "Failed to allocate "
>  			       "memory for Fusion context info\n");
>  			goto fail_alloc_dma_buf;
>  		}
> @@ -5243,7 +5243,7 @@ static int megasas_probe_one(struct pci_dev
> *pdev,
>  					     &instance->consumer_h);
>  
>  		if (!instance->producer || !instance->consumer) {
> -			dev_printk(KERN_DEBUG, &pdev->dev, "Failed
> to allocate"
> +			dev_dbg(&pdev->dev, "Failed to allocate"
>  			       "memory for producer, consumer\n");
>  			goto fail_alloc_dma_buf;
>  		}
> @@ -5290,7 +5290,7 @@ static int megasas_probe_one(struct pci_dev
> *pdev,
>  						    &instance-
> >evt_detail_h);
>  
>  	if (!instance->evt_detail) {
> -		dev_printk(KERN_DEBUG, &pdev->dev, "Failed to
> allocate memory for "
> +		dev_dbg(&pdev->dev, "Failed to allocate memory for "
>  		       "event detail structure\n");
>  		goto fail_alloc_dma_buf;
>  	}
> @@ -5397,7 +5397,7 @@ static int megasas_probe_one(struct pci_dev
> *pdev,
>  	 * Initiate AEN (Asynchronous Event Notification)
>  	 */
>  	if (megasas_start_aen(instance)) {
> -		dev_printk(KERN_DEBUG, &pdev->dev, "start aen
> failed\n");
> +		dev_dbg(&pdev->dev, "start aen failed\n");
>  		goto fail_start_aen;
>  	}
>  
> @@ -5973,14 +5973,14 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> *instance,
>  	memset(kbuff_arr, 0, sizeof(kbuff_arr));
>  
>  	if (ioc->sge_count > MAX_IOCTL_SGE) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "SGE
> count [%d] >  max limit [%d]\n",
> +		dev_dbg(&instance->pdev->dev, "SGE count [%d] >  max
> limit [%d]\n",
>  		       ioc->sge_count, MAX_IOCTL_SGE);
>  		return -EINVAL;
>  	}
>  
>  	cmd = megasas_get_cmd(instance);
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Failed
> to get a cmd packet\n");
> +		dev_dbg(&instance->pdev->dev, "Failed to get a cmd
> packet\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -6025,7 +6025,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> *instance,
>  						    ioc-
> >sgl[i].iov_len,
>  						    &buf_handle,
> GFP_KERNEL);
>  		if (!kbuff_arr[i]) {
> -			dev_printk(KERN_DEBUG, &instance->pdev->dev, 
> "Failed to alloc "
> +			dev_dbg(&instance->pdev->dev, "Failed to
> alloc "
>  			       "kernel SGL buffer for IOCTL\n");
>  			error = -ENOMEM;
>  			goto out;
> @@ -6111,7 +6111,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> *instance,
>  	 */
>  	if (copy_to_user(&user_ioc->frame.hdr.cmd_status,
>  			 &cmd->frame->hdr.cmd_status, sizeof(u8))) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "Error
> copying out cmd_status\n");
> +		dev_dbg(&instance->pdev->dev, "Error copying out
> cmd_status\n");
>  		error = -EFAULT;
>  	}
>  

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

RE: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Kashyap Desai]

> > +		dev_dbg(&instance->pdev->dev, "Error copying out
> > cmd_status\n");
> >  		error = -EFAULT;
> >  	}
> >
>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

We will consider all three patches for future submission. As of now we have
two patch set pending to be committed.
We are working for few more patch in megaraid_sas which will do clean up in
driver module + proper error handling of DCMD command timeout. It will cover
Patch posted with below subject -

[PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames
failed

James  -  We will be resending these patch set on top of latest outstanding
megaraid_sas driver patch, so that we can avoid any conflict in commits.

Re: [PATCH 2/3] megaraid_sas: Convert printk to printk_<level>

[Joe Perches]

On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
> Reduce object size a little by using pr_<level>
> calls instead of printk(KERN_<LEVEL>.

pr_debug does not behave the same as printk(KERN_DEBUG

pr_debug is only active when DEBUG is #defined or dynamic_debug
is enabled.

printk(KERN_DEBUG is always emitted as long as the debug level
is enabled for the console.

At a minimum, your commit message should show you know that.

> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
[]
> @@ -5889,7 +5889,7 @@ static int megasas_mgmt_fasync(int fd, struct file *filep, int mode)
>  		return 0;
>  	}
>  
> -	printk(KERN_DEBUG "megasas: fasync_helper failed [%d]\n", rc);
> +	pr_debug("megasas: fasync_helper failed [%d]\n", rc);

[]

> @@ -6233,7 +6233,7 @@ static int megasas_mgmt_ioctl_aen(struct file *file, unsigned long arg)
>  	u32 wait_time = MEGASAS_RESET_WAIT_TIME;
>  
>  	if (file->private_data != file) {
> -		printk(KERN_DEBUG "megasas: fasync_helper was not "
> +		pr_debug("megasas: fasync_helper was not "
>  		       "called first\n");

Please also coalesce format strings where possible.

Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Joe Perches]

On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
> Reduce object size a little by using dev_<level>
> calls instead of dev_printk(KERN_<LEVEL>.

This is also not the same output.

dev_printk(KERN_DEBUG vs dev_dbg has the same
behavior as printk(KERN_DEBUG vs pr_debug

> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
[]
> @@ -1884,7 +1884,7 @@ static int megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
>  	cmd = megasas_get_cmd(instance);
>  
>  	if (!cmd) {
> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
> +		dev_dbg(&instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
>  		       "Failed to get cmd for scsi%d\n",
>  			instance->host->host_no);
>  		return -ENOMEM;
[]
> @@ -5243,7 +5243,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
>  					     &instance->consumer_h);
>  
>  		if (!instance->producer || !instance->consumer) {
> -			dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate"
> +			dev_dbg(&pdev->dev, "Failed to allocate"
>  			       "memory for producer, consumer\n");

Note the lack of a space between coalesced string segment words.
That's one of the reasons to coalesce them.

Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Weidong Wang]

On 2015/10/28 3:35, Joe Perches wrote:
> On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
>> Reduce object size a little by using dev_<level>
>> calls instead of dev_printk(KERN_<LEVEL>.
> 
> This is also not the same output.
> 
> dev_printk(KERN_DEBUG vs dev_dbg has the same
> behavior as printk(KERN_DEBUG vs pr_debug
> 

yep, You are right.

As Kashyap said, these two patches(Covert [dev_]printk to [dev|pr]_<level>) may
introduced conflicts to their working for megaraid_sas.
So just ignore these two patches.

Regards,
Weidong

>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> []
>> @@ -1884,7 +1884,7 @@ static int megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance,
>>  	cmd = megasas_get_cmd(instance);
>>  
>>  	if (!cmd) {
>> -		dev_printk(KERN_DEBUG, &instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
>> +		dev_dbg(&instance->pdev->dev, "megasas_get_ld_vf_affiliation_111:"
>>  		       "Failed to get cmd for scsi%d\n",
>>  			instance->host->host_no);
>>  		return -ENOMEM;
> []
>> @@ -5243,7 +5243,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
>>  					     &instance->consumer_h);
>>  
>>  		if (!instance->producer || !instance->consumer) {
>> -			dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate"
>> +			dev_dbg(&pdev->dev, "Failed to allocate"
>>  			       "memory for producer, consumer\n");
> 
> Note the lack of a space between coalesced string segment words.
> That's one of the reasons to coalesce them.
> 
> 
> 
> .
> 

Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>

[Weidong Wang]

On 2015/10/27 18:33, Kashyap Desai wrote:
>>> +		dev_dbg(&instance->pdev->dev, "Error copying out
>>> cmd_status\n");
>>>  		error = -EFAULT;
>>>  	}
>>>
>>
>> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> 
> We will consider all three patches for future submission. As of now we have
> two patch set pending to be committed.
> We are working for few more patch in megaraid_sas which will do clean up in
> driver module + proper error handling of DCMD command timeout. It will cover
> Patch posted with below subject -
> 
> [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames
> failed
> 

Ok. And that, can you add Signed-off-by with me as well?

Regards,
Weidong

> James  -  We will be resending these patch set on top of latest outstanding
> megaraid_sas driver patch, so that we can avoid any conflict in commits.
>
>