* [PATCH 0/3] megaraid_sas: copule of fixes
@ 2015-10-27 8:26 Weidong Wang
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Weidong Wang @ 2015-10-27 8:26 UTC (permalink / raw)
To: kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, wangweidong1
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 8:26 [PATCH 0/3] megaraid_sas: copule of fixes Weidong Wang
@ 2015-10-27 8:26 ` Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 19:35 ` Joe Perches
2015-10-27 8:26 ` [PATCH 2/3] megaraid_sas: Convert printk to printk_<level> Weidong Wang
2015-10-27 8:26 ` [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed Weidong Wang
2 siblings, 2 replies; 12+ messages in thread
From: Weidong Wang @ 2015-10-27 8:26 UTC (permalink / raw)
To: kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, wangweidong1
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] megaraid_sas: Convert printk to printk_<level>
2015-10-27 8:26 [PATCH 0/3] megaraid_sas: copule of fixes Weidong Wang
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
@ 2015-10-27 8:26 ` Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 19:32 ` Joe Perches
2015-10-27 8:26 ` [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed Weidong Wang
2 siblings, 2 replies; 12+ messages in thread
From: Weidong Wang @ 2015-10-27 8:26 UTC (permalink / raw)
To: kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, wangweidong1
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed
2015-10-27 8:26 [PATCH 0/3] megaraid_sas: copule of fixes Weidong Wang
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
2015-10-27 8:26 ` [PATCH 2/3] megaraid_sas: Convert printk to printk_<level> Weidong Wang
@ 2015-10-27 8:26 ` Weidong Wang
2015-10-27 10:17 ` Johannes Thumshirn
2 siblings, 1 reply; 12+ messages in thread
From: Weidong Wang @ 2015-10-27 8:26 UTC (permalink / raw)
To: kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, wangweidong1
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed
2015-10-27 8:26 ` [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed Weidong Wang
@ 2015-10-27 10:17 ` Johannes Thumshirn
0 siblings, 0 replies; 12+ messages in thread
From: Johannes Thumshirn @ 2015-10-27 10:17 UTC (permalink / raw)
To: Weidong Wang, kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] megaraid_sas: Convert printk to printk_<level>
2015-10-27 8:26 ` [PATCH 2/3] megaraid_sas: Convert printk to printk_<level> Weidong Wang
@ 2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 19:32 ` Joe Perches
1 sibling, 0 replies; 12+ messages in thread
From: Johannes Thumshirn @ 2015-10-27 10:25 UTC (permalink / raw)
To: Weidong Wang, kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
@ 2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 10:33 ` Kashyap Desai
2015-10-27 19:35 ` Joe Perches
1 sibling, 1 reply; 12+ messages in thread
From: Johannes Thumshirn @ 2015-10-27 10:25 UTC (permalink / raw)
To: Weidong Wang, kashyap.desai, sumit.saxena, uday.lingala, JBottomley
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 10:25 ` Johannes Thumshirn
@ 2015-10-27 10:33 ` Kashyap Desai
2015-10-28 2:31 ` Weidong Wang
0 siblings, 1 reply; 12+ messages in thread
From: Kashyap Desai @ 2015-10-27 10:33 UTC (permalink / raw)
To: Johannes Thumshirn, Weidong Wang, Sumit Saxena, Uday Lingala, JBottomley
Cc: PDL,MEGARAIDLINUX, linux-scsi, linux-kernel
> > + 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] megaraid_sas: Convert printk to printk_<level>
2015-10-27 8:26 ` [PATCH 2/3] megaraid_sas: Convert printk to printk_<level> Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
@ 2015-10-27 19:32 ` Joe Perches
1 sibling, 0 replies; 12+ messages in thread
From: Joe Perches @ 2015-10-27 19:32 UTC (permalink / raw)
To: Weidong Wang
Cc: kashyap.desai, sumit.saxena, uday.lingala, JBottomley,
megaraidlinux.pdl, linux-scsi, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
@ 2015-10-27 19:35 ` Joe Perches
2015-10-28 1:59 ` Weidong Wang
1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2015-10-27 19:35 UTC (permalink / raw)
To: Weidong Wang
Cc: kashyap.desai, sumit.saxena, uday.lingala, JBottomley,
megaraidlinux.pdl, linux-scsi, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 19:35 ` Joe Perches
@ 2015-10-28 1:59 ` Weidong Wang
0 siblings, 0 replies; 12+ messages in thread
From: Weidong Wang @ 2015-10-28 1:59 UTC (permalink / raw)
To: Joe Perches
Cc: kashyap.desai, sumit.saxena, uday.lingala, JBottomley,
megaraidlinux.pdl, linux-scsi, linux-kernel
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.
>
>
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level>
2015-10-27 10:33 ` Kashyap Desai
@ 2015-10-28 2:31 ` Weidong Wang
0 siblings, 0 replies; 12+ messages in thread
From: Weidong Wang @ 2015-10-28 2:31 UTC (permalink / raw)
To: Kashyap Desai
Cc: Johannes Thumshirn, Sumit Saxena, Uday Lingala, JBottomley,
PDL,MEGARAIDLINUX, linux-scsi, linux-kernel
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.
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-10-28 2:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-27 8:26 [PATCH 0/3] megaraid_sas: copule of fixes Weidong Wang
2015-10-27 8:26 ` [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_<level> Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 10:33 ` Kashyap Desai
2015-10-28 2:31 ` Weidong Wang
2015-10-27 19:35 ` Joe Perches
2015-10-28 1:59 ` Weidong Wang
2015-10-27 8:26 ` [PATCH 2/3] megaraid_sas: Convert printk to printk_<level> Weidong Wang
2015-10-27 10:25 ` Johannes Thumshirn
2015-10-27 19:32 ` Joe Perches
2015-10-27 8:26 ` [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed Weidong Wang
2015-10-27 10:17 ` Johannes Thumshirn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).