| Message ID | 20221019024220.376178-4-shaozhengchao@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | fix some issues in Huawei hinic driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 17 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote: > When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is > not released correctly. Fix it. > > Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > --- > drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ > 1 file changed, 5 insertions(+) <...> > + cmdq_type = HINIC_CMDQ_SYNC; > + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) Why do you have this "for loops" in all places? There is only one cmdq_type. Thanks
On 2022/10/19 15:20, Leon Romanovsky wrote: > On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote: >> When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is >> not released correctly. Fix it. >> >> Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") >> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> >> --- >> drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ >> 1 file changed, 5 insertions(+) > > <...> > >> + cmdq_type = HINIC_CMDQ_SYNC; >> + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) > > Why do you have this "for loops" in all places? There is only one cmdq_type. > > Thanks Hi Leon: Thank you for your review. Now, only the synchronous CMDQ is enabled for the current CMDQs. New type of CMDQ could be added later. So looping style is maintained on both the allocation and release paths. Zhengchao Shao
On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote: > > > On 2022/10/19 15:20, Leon Romanovsky wrote: > > On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote: > > > When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is > > > not released correctly. Fix it. > > > > > > Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") > > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > > > --- > > > drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > <...> > > > > > + cmdq_type = HINIC_CMDQ_SYNC; > > > + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) > > > > Why do you have this "for loops" in all places? There is only one cmdq_type. > > > > Thanks > Hi Leon: > Thank you for your review. Now, only the synchronous CMDQ is > enabled for the current CMDQs. New type of CMDQ could be added later. Single command type was added in 2017, and five years later, new type wasn't added yet. > So looping style is maintained on both the allocation and release paths. > > Zhengchao Shao
On 2022/10/19 16:39, Leon Romanovsky wrote: > On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote: >> >> >> On 2022/10/19 15:20, Leon Romanovsky wrote: >>> On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote: >>>> When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is >>>> not released correctly. Fix it. >>>> >>>> Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") >>>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> >>>> --- >>>> drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>> >>> <...> >>> >>>> + cmdq_type = HINIC_CMDQ_SYNC; >>>> + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) >>> >>> Why do you have this "for loops" in all places? There is only one cmdq_type. >>> >>> Thanks >> Hi Leon: >> Thank you for your review. Now, only the synchronous CMDQ is >> enabled for the current CMDQs. New type of CMDQ could be added later. > > Single command type was added in 2017, and five years later, new type wasn't added yet. > OK, I will modify in V2, and I will do cleanup in another patch. Thanks Zhengchao Shao >> So looping style is maintained on both the allocation and release paths. >> >> Zhengchao Shao
On Wed, Oct 19, 2022 at 05:37:42PM +0800, shaozhengchao wrote: > > > On 2022/10/19 16:39, Leon Romanovsky wrote: > > On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote: > > > > > > > > > On 2022/10/19 15:20, Leon Romanovsky wrote: > > > > On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote: > > > > > When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is > > > > > not released correctly. Fix it. > > > > > > > > > > Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") > > > > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > > > > > --- > > > > > drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ > > > > > 1 file changed, 5 insertions(+) > > > > > > > > <...> > > > > > > > > > + cmdq_type = HINIC_CMDQ_SYNC; > > > > > + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) > > > > > > > > Why do you have this "for loops" in all places? There is only one cmdq_type. > > > > > > > > Thanks > > > Hi Leon: > > > Thank you for your review. Now, only the synchronous CMDQ is > > > enabled for the current CMDQs. New type of CMDQ could be added later. > > > > Single command type was added in 2017, and five years later, new type wasn't added yet. > > > OK, I will modify in V2, and I will do cleanup in another patch. Thanks > > Thanks > > Zhengchao Shao > > > > So looping style is maintained on both the allocation and release paths. > > > > > > Zhengchao Shao
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c index 78190e88cd75..2a759b9bb6b6 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c @@ -877,6 +877,7 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif, { struct hinic_func_to_io *func_to_io = cmdqs_to_func_to_io(cmdqs); struct pci_dev *pdev = hwif->pdev; + enum hinic_cmdq_type cmdq_type; struct hinic_hwdev *hwdev; u16 max_wqe_size; int err; @@ -925,6 +926,10 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif, err_set_cmdq_depth: hinic_ceq_unregister_cb(&func_to_io->ceqs, HINIC_CEQ_CMDQ); + cmdq_type = HINIC_CMDQ_SYNC; + for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++) + free_cmdq(&cmdqs->cmdq[cmdq_type]); + err_cmdq_ctxt: hinic_wqs_cmdq_free(&cmdqs->cmdq_pages, cmdqs->saved_wqs, HINIC_MAX_CMDQ_TYPES);
When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is not released correctly. Fix it. Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf") Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> --- drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++ 1 file changed, 5 insertions(+)