| Message ID | db0c6205-9ca2-d80e-26fc-6cff0d84d73c@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Mon, Sep 26, 2016 at 03:36:25PM -0700, Bart Van Assche wrote: > Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register() > such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch > avoids that smatch reports the following error: > > block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex' > > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> > Cc: Tejun Heo <tj@kernel.org> > Cc: <stable@vger.kernel.org> > --- > block/blk-cgroup.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index dd38e5c..cdbca1c 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -1327,8 +1327,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) > for (i = 0; i < BLKCG_MAX_POLS; i++) > if (!blkcg_policy[i]) > break; > - if (i >= BLKCG_MAX_POLS) > + if (i >= BLKCG_MAX_POLS) { > + mutex_unlock(&blkcg_pol_mutex); > goto err_unlock; > + } Wouldn't it be better to drop explicit mutx_unlock(&blkcg_pol_mutex) on "if (!cpd)"? Thanks.
On 09/29/2016 03:03 AM, Tejun Heo wrote: > Hello, > > On Mon, Sep 26, 2016 at 03:36:25PM -0700, Bart Van Assche wrote: >> Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register() >> such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch >> avoids that smatch reports the following error: >> >> block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex' >> >> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> >> Cc: Tejun Heo <tj@kernel.org> >> Cc: <stable@vger.kernel.org> >> --- >> block/blk-cgroup.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c >> index dd38e5c..cdbca1c 100644 >> --- a/block/blk-cgroup.c >> +++ b/block/blk-cgroup.c >> @@ -1327,8 +1327,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) >> for (i = 0; i < BLKCG_MAX_POLS; i++) >> if (!blkcg_policy[i]) >> break; >> - if (i >= BLKCG_MAX_POLS) >> + if (i >= BLKCG_MAX_POLS) { >> + mutex_unlock(&blkcg_pol_mutex); >> goto err_unlock; >> + } > > Wouldn't it be better to drop explicit mutx_unlock(&blkcg_pol_mutex) > on "if (!cpd)"? Agreed. I will rework this patch accordingly. Thanks for the feedback. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index dd38e5c..cdbca1c 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1327,8 +1327,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) for (i = 0; i < BLKCG_MAX_POLS; i++) if (!blkcg_policy[i]) break; - if (i >= BLKCG_MAX_POLS) + if (i >= BLKCG_MAX_POLS) { + mutex_unlock(&blkcg_pol_mutex); goto err_unlock; + } /* register @pol */ pol->plid = i; @@ -1374,8 +1376,8 @@ err_free_cpds: } } blkcg_policy[pol->plid] = NULL; + err_unlock: - mutex_unlock(&blkcg_pol_mutex); mutex_unlock(&blkcg_pol_register_mutex); return ret; }
Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register() such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch avoids that smatch reports the following error: block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex' Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Cc: Tejun Heo <tj@kernel.org> Cc: <stable@vger.kernel.org> --- block/blk-cgroup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)