| Message ID | 20200921131039.92249-1-miaoqinglang@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [-next] mlxsw: spectrum_acl_tcam: simplify the return expression of ishtp_cl_driver_register() | expand |
On Mon, Sep 21, 2020 at 09:10:39PM +0800, Qinglang Miao wrote: > Simplify the return expression. > > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> > --- > drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > index 5c0204033..5b4313991 100644 > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > @@ -289,17 +289,11 @@ static int > mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp_acl_tcam *tcam, > struct mlxsw_sp_acl_tcam_group *group) > { > - int err; > - > group->tcam = tcam; > mutex_init(&group->lock); > INIT_LIST_HEAD(&group->region_list); > > - err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); > - if (err) > - return err; > - > - return 0; > + return mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); There is actually a problem here. We don't call mutex_destroy() on error. Should be: diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c index 5c020403342f..7cccc41dd69c 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c @@ -292,13 +292,14 @@ mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp_acl_tcam *tcam, int err; group->tcam = tcam; - mutex_init(&group->lock); INIT_LIST_HEAD(&group->region_list); err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); if (err) return err; + mutex_init(&group->lock); + return 0; } Then it's symmetric with mlxsw_sp_acl_tcam_group_del(). Do you want to send this patch to 'net' or should I? If so, it should have this Fixes line: Fixes: 5ec2ee28d27b ("mlxsw: spectrum_acl: Introduce a mutex to guard region list updates") Thanks > } > > static void mlxsw_sp_acl_tcam_group_del(struct mlxsw_sp_acl_tcam_group *group) > -- > 2.23.0 >
在 2020/9/21 21:51, Ido Schimmel 写道: > On Mon, Sep 21, 2020 at 09:10:39PM +0800, Qinglang Miao wrote: >> Simplify the return expression. >> >> Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> >> --- >> drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c >> index 5c0204033..5b4313991 100644 >> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c >> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c >> @@ -289,17 +289,11 @@ static int >> mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp_acl_tcam *tcam, >> struct mlxsw_sp_acl_tcam_group *group) >> { >> - int err; >> - >> group->tcam = tcam; >> mutex_init(&group->lock); >> INIT_LIST_HEAD(&group->region_list); >> >> - err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); >> - if (err) >> - return err; >> - >> - return 0; >> + return mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); > There is actually a problem here. We don't call mutex_destroy() on > error. Should be: > > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > index 5c020403342f..7cccc41dd69c 100644 > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c > @@ -292,13 +292,14 @@ mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp_acl_tcam *tcam, > int err; > > group->tcam = tcam; > - mutex_init(&group->lock); > INIT_LIST_HEAD(&group->region_list); > > err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); > if (err) > return err; > > + mutex_init(&group->lock); > + > return 0; > } > > Then it's symmetric with mlxsw_sp_acl_tcam_group_del(). Do you want to > send this patch to 'net' or should I? If so, it should have this Fixes > line: > > Fixes: 5ec2ee28d27b ("mlxsw: spectrum_acl: Introduce a mutex to guard region list updates") > > Thanks > Hi Ido, Sorry I didn't realize this problem. As for this bugfix patch, I think there's no doubt that you are the one who should send it :D. Thanks. >> } >> >> static void mlxsw_sp_acl_tcam_group_del(struct mlxsw_sp_acl_tcam_group *group) >> -- >> 2.23.0 >> > . >
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c index 5c0204033..5b4313991 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c @@ -289,17 +289,11 @@ static int mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp_acl_tcam *tcam, struct mlxsw_sp_acl_tcam_group *group) { - int err; - group->tcam = tcam; mutex_init(&group->lock); INIT_LIST_HEAD(&group->region_list); - err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); - if (err) - return err; - - return 0; + return mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id); } static void mlxsw_sp_acl_tcam_group_del(struct mlxsw_sp_acl_tcam_group *group)
Simplify the return expression. Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> --- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)