2015-12-27 05:08:28

by Joshua Clayton

[permalink] [raw]
Subject: [PATCH] staging: lustre: fix lock imbalance

nrs_resource_put_safe() might hold a lock one one struct
while operating on the other.
There are 2 levels of structures.
Use nrs_policy_put(), which has locking baked in.

sparse gives the following warning:
drivers/staging/lustre//lustre/ptlrpc/nrs.c:498:39:
warning: context imbalance in 'nrs_resource_put_safe' -
different lock contexts for basic block
---
drivers/staging/lustre/lustre/ptlrpc/nrs.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
index 7044e1f..9028b12 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -496,18 +496,9 @@ static void nrs_resource_put_safe(struct ptlrpc_nrs_resource **resp)
}

for (i = 0; i < NRS_RES_MAX; i++) {
- if (pols[i] == NULL)
- continue;
-
- if (nrs == NULL) {
- nrs = pols[i]->pol_nrs;
- spin_lock(&nrs->nrs_lock);
- }
- nrs_policy_put_locked(pols[i]);
+ if (pols[i])
+ nrs_policy_put(pols[i]);
}
-
- if (nrs != NULL)
- spin_unlock(&nrs->nrs_lock);
}

/**
--
2.6.4


2015-12-27 05:34:14

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] staging: lustre: fix lock imbalance

[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
Hi Joshua,

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.4-rc6 next-20151223]

url: https://github.com/0day-ci/linux/commits/Joshua-Clayton/staging-lustre-fix-lock-imbalance/20151227-131137
config: x86_64-allyesconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers/staging/lustre/lustre/ptlrpc/nrs.c: In function 'nrs_resource_put_safe':
>> drivers/staging/lustre/lustre/ptlrpc/nrs.c:485:21: warning: unused variable 'nrs' [-Wunused-variable]
struct ptlrpc_nrs *nrs = NULL;
^

vim +/nrs +485 drivers/staging/lustre/lustre/ptlrpc/nrs.c

d7e09d039 Peng Tao 2013-05-02 469 nrs_policy_put(primary);
d7e09d039 Peng Tao 2013-05-02 470 }
d7e09d039 Peng Tao 2013-05-02 471 }
d7e09d039 Peng Tao 2013-05-02 472
d7e09d039 Peng Tao 2013-05-02 473 /**
d7e09d039 Peng Tao 2013-05-02 474 * Releases references to resource hierarchies and policies, because they are no
d7e09d039 Peng Tao 2013-05-02 475 * longer required; used when request handling has been completed, or the
d7e09d039 Peng Tao 2013-05-02 476 * request is moving to the high priority NRS head.
d7e09d039 Peng Tao 2013-05-02 477 *
d7e09d039 Peng Tao 2013-05-02 478 * \param resp the resource hierarchy that is being released
d7e09d039 Peng Tao 2013-05-02 479 *
d7e09d039 Peng Tao 2013-05-02 480 * \see ptlrpc_nrs_req_finalize()
d7e09d039 Peng Tao 2013-05-02 481 */
d7e09d039 Peng Tao 2013-05-02 482 static void nrs_resource_put_safe(struct ptlrpc_nrs_resource **resp)
d7e09d039 Peng Tao 2013-05-02 483 {
d7e09d039 Peng Tao 2013-05-02 484 struct ptlrpc_nrs_policy *pols[NRS_RES_MAX];
d7e09d039 Peng Tao 2013-05-02 @485 struct ptlrpc_nrs *nrs = NULL;
d7e09d039 Peng Tao 2013-05-02 486 int i;
d7e09d039 Peng Tao 2013-05-02 487
d7e09d039 Peng Tao 2013-05-02 488 for (i = 0; i < NRS_RES_MAX; i++) {
d7e09d039 Peng Tao 2013-05-02 489 if (resp[i] != NULL) {
d7e09d039 Peng Tao 2013-05-02 490 pols[i] = resp[i]->res_policy;
d7e09d039 Peng Tao 2013-05-02 491 nrs_resource_put(resp[i]);
d7e09d039 Peng Tao 2013-05-02 492 resp[i] = NULL;
d7e09d039 Peng Tao 2013-05-02 493 } else {

:::::: The code at line 485 was first introduced by commit
:::::: d7e09d0397e84eefbabfd9cb353221f3c6448d83 staging: add Lustre file system client support

:::::: TO: Peng Tao <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.74 kB)
.config.gz (50.21 kB)
Download all attachments

2015-12-27 06:48:53

by Joshua Clayton

[permalink] [raw]
Subject: [PATCH v2] staging: lustre: fix lock imbalance

nrs_resource_put_safe() might hold a lock one one struct
while operating on the other.
There are 2 levels of structures.
Use nrs_policy_put(), which has locking baked in.

sparse gives the following warning:
drivers/staging/lustre//lustre/ptlrpc/nrs.c:498:39:
warning: context imbalance in 'nrs_resource_put_safe' -
different lock contexts for basic block

Signed-off-by: Joshua Clayton <[email protected]>
---
changed for v2:
- remove unused nrs variable

drivers/staging/lustre/lustre/ptlrpc/nrs.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
index 7044e1f..57acf8c 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -482,7 +482,6 @@ static void nrs_resource_get_safe(struct ptlrpc_nrs *nrs,
static void nrs_resource_put_safe(struct ptlrpc_nrs_resource **resp)
{
struct ptlrpc_nrs_policy *pols[NRS_RES_MAX];
- struct ptlrpc_nrs *nrs = NULL;
int i;

for (i = 0; i < NRS_RES_MAX; i++) {
@@ -496,18 +495,9 @@ static void nrs_resource_put_safe(struct ptlrpc_nrs_resource **resp)
}

for (i = 0; i < NRS_RES_MAX; i++) {
- if (pols[i] == NULL)
- continue;
-
- if (nrs == NULL) {
- nrs = pols[i]->pol_nrs;
- spin_lock(&nrs->nrs_lock);
- }
- nrs_policy_put_locked(pols[i]);
+ if (pols[i])
+ nrs_policy_put(pols[i]);
}
-
- if (nrs != NULL)
- spin_unlock(&nrs->nrs_lock);
}

/**
--
2.6.4