Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 18, 2016, at 14:21, nayeem  wrote:
> On Friday 16 September 2016 01:30 PM, Dilger, Andreas wrote:
>> On Sep 15, 2016, at 12:33, nayeem  wrote:
>>> On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:
 On Sep 12, 2016, at 04:27, Greg KH  wrote:
> 
> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
>> Added __acquires / __releases sparse locking annotations
>> to lock_res_and_lock and unlock_res_and_lock functions in
>> l_lock.c, to fix below sparse warnings:
>> 
>> l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - 
>> wrong count at exit
>> l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
>> unexpected unlock
>> 
>> Signed-off-by: Nayeemahmed Badebade 
>> ---
>> drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
>> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> index ea8840c..c4b9612 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> @@ -45,6 +45,8 @@
>>  * being an atomic operation.
>>  */
>> struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
>> +__acquires(>l_lock)
>> +__acquires(lock->l_resource)
> 
> Hm, these are tricky, I don't want to take this type of change without
> an ack from the lustre developers...
 
 The "__acquires(>l_lock)" line here looks correct, along with the
 corresponding "__releases(>l_lock)" at unlock_res_and_lock().
 
 The problem, however, is that "l_resource" is not a lock, but rather a
 struct.  The call to "lock_res(lock->l_resource)" is actually locking
 "lr_lock" internally.
 
 It would be better to add "__acquires(>lr_lock)" at lock_res() and
 "__releases(>lr_lock)" at unlock_res().  That will also forestall
 any other warnings about an imbalance with lock_res()/unlock_res() or
 their callsites.
 
 Cheers, Andreas
 
>>> 
>>> Hi Andreas,
>>> 
>>> Thank you for your review comments. I did the change according to your 
>>> comments and the diff is attached to mail. But this change doesn't seem to 
>>> fix the sparse warning.
>>> With this change when i compile the code "make C=2 
>>> ./drivers/staging/lustre/lustre/", sparse warning still comes:
>> 
>>> {{{
>>>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>>> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
>>> warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
>>> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
>>> warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock
>>>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
>>> }}}
>> 
>> Strange, one would think that your patch should work properly.  Maybe the
>> __acquires() label doesn't work on inline functions?
>> 
> 
> I think sparse works on inline functions.
> I ran sparse on a hello world kernel module in different cases explained below
> 
> 
>>> Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & 
>>> "__acquires(>l_lock)" at lock_res_and_lock() and 
>>> "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at 
>>> unlock_res_and_lock() ?
>>> Because with that change the sparse warning is fixed.
>>> {{{
>>>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>>>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
>>> }}}
>> 
>> This would also be possible, but then it exposes any callers of lock_res()
>> and unlock() res to similar compiler warnings in the future.  I'm not
>> against this in principle, but it is worthwhile to see why sparse is not
>> handling this case correctly.
>> 
>> Cheers, Andreas
>> 
> 
> case 1:
> ---
> hello.c, where spin_lock() and spin_unlock() are called indirectly via 
> foo_lock() and foo_unlock() in the same function i.e "say_hello()" in below 
> code.
> 
> The following code when checked with sparse doesn't give any warning
> 
> #include
> #include
> 
> static DEFINE_SPINLOCK(my_lock);
> 
> static inline void foo_lock(spinlock_t *spl)
> {
>spin_lock(spl);
> }
> 
> static inline void foo_unlock(spinlock_t *spl)
> {
>spin_unlock(spl);
> }
> 
> static int __init say_hello(void)
> {
>foo_lock(_lock);
>pr_info("Hello World!\n");
>foo_unlock(_lock);
>return 0;
> }
> 
> static void __exit cleanup(void)
> {
> }
> 
> module_init(say_hello);
> module_exit(cleanup);
> 
> 
> 
> case 2.
> --
> The above code when slightly modified so that, spin_lock() is called 
> indirectly via foo_lock() in say_hello() 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 18, 2016, at 14:21, nayeem  wrote:
> On Friday 16 September 2016 01:30 PM, Dilger, Andreas wrote:
>> On Sep 15, 2016, at 12:33, nayeem  wrote:
>>> On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:
 On Sep 12, 2016, at 04:27, Greg KH  wrote:
> 
> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
>> Added __acquires / __releases sparse locking annotations
>> to lock_res_and_lock and unlock_res_and_lock functions in
>> l_lock.c, to fix below sparse warnings:
>> 
>> l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - 
>> wrong count at exit
>> l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
>> unexpected unlock
>> 
>> Signed-off-by: Nayeemahmed Badebade 
>> ---
>> drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
>> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> index ea8840c..c4b9612 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> @@ -45,6 +45,8 @@
>>  * being an atomic operation.
>>  */
>> struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
>> +__acquires(>l_lock)
>> +__acquires(lock->l_resource)
> 
> Hm, these are tricky, I don't want to take this type of change without
> an ack from the lustre developers...
 
 The "__acquires(>l_lock)" line here looks correct, along with the
 corresponding "__releases(>l_lock)" at unlock_res_and_lock().
 
 The problem, however, is that "l_resource" is not a lock, but rather a
 struct.  The call to "lock_res(lock->l_resource)" is actually locking
 "lr_lock" internally.
 
 It would be better to add "__acquires(>lr_lock)" at lock_res() and
 "__releases(>lr_lock)" at unlock_res().  That will also forestall
 any other warnings about an imbalance with lock_res()/unlock_res() or
 their callsites.
 
 Cheers, Andreas
 
>>> 
>>> Hi Andreas,
>>> 
>>> Thank you for your review comments. I did the change according to your 
>>> comments and the diff is attached to mail. But this change doesn't seem to 
>>> fix the sparse warning.
>>> With this change when i compile the code "make C=2 
>>> ./drivers/staging/lustre/lustre/", sparse warning still comes:
>> 
>>> {{{
>>>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>>> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
>>> warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
>>> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
>>> warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock
>>>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
>>> }}}
>> 
>> Strange, one would think that your patch should work properly.  Maybe the
>> __acquires() label doesn't work on inline functions?
>> 
> 
> I think sparse works on inline functions.
> I ran sparse on a hello world kernel module in different cases explained below
> 
> 
>>> Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & 
>>> "__acquires(>l_lock)" at lock_res_and_lock() and 
>>> "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at 
>>> unlock_res_and_lock() ?
>>> Because with that change the sparse warning is fixed.
>>> {{{
>>>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>>>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
>>> }}}
>> 
>> This would also be possible, but then it exposes any callers of lock_res()
>> and unlock() res to similar compiler warnings in the future.  I'm not
>> against this in principle, but it is worthwhile to see why sparse is not
>> handling this case correctly.
>> 
>> Cheers, Andreas
>> 
> 
> case 1:
> ---
> hello.c, where spin_lock() and spin_unlock() are called indirectly via 
> foo_lock() and foo_unlock() in the same function i.e "say_hello()" in below 
> code.
> 
> The following code when checked with sparse doesn't give any warning
> 
> #include
> #include
> 
> static DEFINE_SPINLOCK(my_lock);
> 
> static inline void foo_lock(spinlock_t *spl)
> {
>spin_lock(spl);
> }
> 
> static inline void foo_unlock(spinlock_t *spl)
> {
>spin_unlock(spl);
> }
> 
> static int __init say_hello(void)
> {
>foo_lock(_lock);
>pr_info("Hello World!\n");
>foo_unlock(_lock);
>return 0;
> }
> 
> static void __exit cleanup(void)
> {
> }
> 
> module_init(say_hello);
> module_exit(cleanup);
> 
> 
> 
> case 2.
> --
> The above code when slightly modified so that, spin_lock() is called 
> indirectly via foo_lock() in say_hello() and spin_unlock() via foo_unlock() 
> in cleanup()
> 
> static int __init say_hello(void)
> {
> 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[nayeem]

On Friday 16 September 2016 01:30 PM, Dilger, Andreas wrote:

On Sep 15, 2016, at 12:33, nayeem  wrote:

On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:

On Sep 12, 2016, at 04:27, Greg KH  wrote:


On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong count 
at exit
l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - unexpected 
unlock

Signed-off-by: Nayeemahmed Badebade 
---
drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)


Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...


The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas



Hi Andreas,

Thank you for your review comments. I did the change according to your comments 
and the diff is attached to mail. But this change doesn't seem to fix the 
sparse warning.
With this change when i compile the code "make C=2 
./drivers/staging/lustre/lustre/", sparse warning still comes:



{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: warning: 
context imbalance in 'lock_res_and_lock' - wrong count at exit
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: warning: 
context imbalance in 'unlock_res_and_lock' - unexpected unlock
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}


Strange, one would think that your patch should work properly.  Maybe the
__acquires() label doesn't work on inline functions?



I think sparse works on inline functions.
I ran sparse on a hello world kernel module in different cases explained 
below




Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & "__acquires(>l_lock)" at 
lock_res_and_lock() and "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at unlock_res_and_lock() ?
Because with that change the sparse warning is fixed.
{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}


This would also be possible, but then it exposes any callers of lock_res()
and unlock() res to similar compiler warnings in the future.  I'm not
against this in principle, but it is worthwhile to see why sparse is not
handling this case correctly.

Cheers, Andreas



case 1:
---
hello.c, where spin_lock() and spin_unlock() are called indirectly via 
foo_lock() and foo_unlock() in the same function i.e "say_hello()" in 
below code.


The following code when checked with sparse doesn't give any warning

#include
#include

static DEFINE_SPINLOCK(my_lock);

static inline void foo_lock(spinlock_t *spl)
{
spin_lock(spl);
}

static inline void foo_unlock(spinlock_t *spl)
{
spin_unlock(spl);
}

static int __init say_hello(void)
{
foo_lock(_lock);
pr_info("Hello World!\n");
foo_unlock(_lock);
return 0;
}

static void __exit cleanup(void)
{
}

module_init(say_hello);
module_exit(cleanup);



case 2.
--
The above code when slightly modified so that, spin_lock() is called 
indirectly via foo_lock() in say_hello() and spin_unlock() via 
foo_unlock() in cleanup()


static int __init say_hello(void)
{
foo_lock(_lock);
pr_info("Hello World!\n");

return 0;
}

static void __exit cleanup(void)
{
foo_unlock(_lock);
}

Then sparse gives the warning:
{{{
test-module/hello.c:16:19: warning: context imbalance in 'say_hello' - 
wrong count at exit
test-module/hello.c:23:20: warning: context imbalance in 'cleanup' - 
unexpected unlock

}}}
To fix this if we put sparse annotations __acquires() at foo_lock() and 
__releases() at foo_unlock(), then also 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[nayeem]

On Friday 16 September 2016 01:30 PM, Dilger, Andreas wrote:

On Sep 15, 2016, at 12:33, nayeem  wrote:

On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:

On Sep 12, 2016, at 04:27, Greg KH  wrote:


On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong count 
at exit
l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - unexpected 
unlock

Signed-off-by: Nayeemahmed Badebade 
---
drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)


Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...


The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas



Hi Andreas,

Thank you for your review comments. I did the change according to your comments 
and the diff is attached to mail. But this change doesn't seem to fix the 
sparse warning.
With this change when i compile the code "make C=2 
./drivers/staging/lustre/lustre/", sparse warning still comes:



{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: warning: 
context imbalance in 'lock_res_and_lock' - wrong count at exit
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: warning: 
context imbalance in 'unlock_res_and_lock' - unexpected unlock
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}


Strange, one would think that your patch should work properly.  Maybe the
__acquires() label doesn't work on inline functions?



I think sparse works on inline functions.
I ran sparse on a hello world kernel module in different cases explained 
below




Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & "__acquires(>l_lock)" at 
lock_res_and_lock() and "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at unlock_res_and_lock() ?
Because with that change the sparse warning is fixed.
{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}


This would also be possible, but then it exposes any callers of lock_res()
and unlock() res to similar compiler warnings in the future.  I'm not
against this in principle, but it is worthwhile to see why sparse is not
handling this case correctly.

Cheers, Andreas



case 1:
---
hello.c, where spin_lock() and spin_unlock() are called indirectly via 
foo_lock() and foo_unlock() in the same function i.e "say_hello()" in 
below code.


The following code when checked with sparse doesn't give any warning

#include
#include

static DEFINE_SPINLOCK(my_lock);

static inline void foo_lock(spinlock_t *spl)
{
spin_lock(spl);
}

static inline void foo_unlock(spinlock_t *spl)
{
spin_unlock(spl);
}

static int __init say_hello(void)
{
foo_lock(_lock);
pr_info("Hello World!\n");
foo_unlock(_lock);
return 0;
}

static void __exit cleanup(void)
{
}

module_init(say_hello);
module_exit(cleanup);



case 2.
--
The above code when slightly modified so that, spin_lock() is called 
indirectly via foo_lock() in say_hello() and spin_unlock() via 
foo_unlock() in cleanup()


static int __init say_hello(void)
{
foo_lock(_lock);
pr_info("Hello World!\n");

return 0;
}

static void __exit cleanup(void)
{
foo_unlock(_lock);
}

Then sparse gives the warning:
{{{
test-module/hello.c:16:19: warning: context imbalance in 'say_hello' - 
wrong count at exit
test-module/hello.c:23:20: warning: context imbalance in 'cleanup' - 
unexpected unlock

}}}
To fix this if we put sparse annotations __acquires() at foo_lock() and 
__releases() at foo_unlock(), then also sparse warnings comes, which is 
exactly the case with l_lock.c in lustre 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 15, 2016, at 12:33, nayeem  wrote:
> On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:
>> On Sep 12, 2016, at 04:27, Greg KH  wrote:
>>> 
>>> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
 Added __acquires / __releases sparse locking annotations
 to lock_res_and_lock and unlock_res_and_lock functions in
 l_lock.c, to fix below sparse warnings:
 
 l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
 count at exit
 l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
 unexpected unlock
 
 Signed-off-by: Nayeemahmed Badebade 
 ---
 drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
 1 file changed, 4 insertions(+)
 
 diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
 b/drivers/staging/lustre/lustre/ldlm/l_lock.c
 index ea8840c..c4b9612 100644
 --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
 +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
 @@ -45,6 +45,8 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
 +  __acquires(>l_lock)
 +  __acquires(lock->l_resource)
>>> 
>>> Hm, these are tricky, I don't want to take this type of change without
>>> an ack from the lustre developers...
>> 
>> The "__acquires(>l_lock)" line here looks correct, along with the
>> corresponding "__releases(>l_lock)" at unlock_res_and_lock().
>> 
>> The problem, however, is that "l_resource" is not a lock, but rather a
>> struct.  The call to "lock_res(lock->l_resource)" is actually locking
>> "lr_lock" internally.
>> 
>> It would be better to add "__acquires(>lr_lock)" at lock_res() and
>> "__releases(>lr_lock)" at unlock_res().  That will also forestall
>> any other warnings about an imbalance with lock_res()/unlock_res() or
>> their callsites.
>> 
>> Cheers, Andreas
>> 
> 
> Hi Andreas,
> 
> Thank you for your review comments. I did the change according to your 
> comments and the diff is attached to mail. But this change doesn't seem to 
> fix the sparse warning.
> With this change when i compile the code "make C=2 
> ./drivers/staging/lustre/lustre/", sparse warning still comes:

> {{{
>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
> warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
> warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock
>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
> }}}

Strange, one would think that your patch should work properly.  Maybe the
__acquires() label doesn't work on inline functions?

> Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & 
> "__acquires(>l_lock)" at lock_res_and_lock() and 
> "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at 
> unlock_res_and_lock() ?
> Because with that change the sparse warning is fixed.
> {{{
>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
> }}}

This would also be possible, but then it exposes any callers of lock_res()
and unlock() res to similar compiler warnings in the future.  I'm not
against this in principle, but it is worthwhile to see why sparse is not
handling this case correctly.

Cheers, Andreas

> Could you please confirm this.
> 
> Regards,
> Nayeem
> 
> 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 15, 2016, at 12:33, nayeem  wrote:
> On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:
>> On Sep 12, 2016, at 04:27, Greg KH  wrote:
>>> 
>>> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
 Added __acquires / __releases sparse locking annotations
 to lock_res_and_lock and unlock_res_and_lock functions in
 l_lock.c, to fix below sparse warnings:
 
 l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
 count at exit
 l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
 unexpected unlock
 
 Signed-off-by: Nayeemahmed Badebade 
 ---
 drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
 1 file changed, 4 insertions(+)
 
 diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
 b/drivers/staging/lustre/lustre/ldlm/l_lock.c
 index ea8840c..c4b9612 100644
 --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
 +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
 @@ -45,6 +45,8 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
 +  __acquires(>l_lock)
 +  __acquires(lock->l_resource)
>>> 
>>> Hm, these are tricky, I don't want to take this type of change without
>>> an ack from the lustre developers...
>> 
>> The "__acquires(>l_lock)" line here looks correct, along with the
>> corresponding "__releases(>l_lock)" at unlock_res_and_lock().
>> 
>> The problem, however, is that "l_resource" is not a lock, but rather a
>> struct.  The call to "lock_res(lock->l_resource)" is actually locking
>> "lr_lock" internally.
>> 
>> It would be better to add "__acquires(>lr_lock)" at lock_res() and
>> "__releases(>lr_lock)" at unlock_res().  That will also forestall
>> any other warnings about an imbalance with lock_res()/unlock_res() or
>> their callsites.
>> 
>> Cheers, Andreas
>> 
> 
> Hi Andreas,
> 
> Thank you for your review comments. I did the change according to your 
> comments and the diff is attached to mail. But this change doesn't seem to 
> fix the sparse warning.
> With this change when i compile the code "make C=2 
> ./drivers/staging/lustre/lustre/", sparse warning still comes:

> {{{
>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
> warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
> drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
> warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock
>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
> }}}

Strange, one would think that your patch should work properly.  Maybe the
__acquires() label doesn't work on inline functions?

> Would it be a good idea to add "__acquires(>l_resource->lr_lock)" & 
> "__acquires(>l_lock)" at lock_res_and_lock() and 
> "__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" at 
> unlock_res_and_lock() ?
> Because with that change the sparse warning is fixed.
> {{{
>  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
>  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
> }}}

This would also be possible, but then it exposes any callers of lock_res()
and unlock() res to similar compiler warnings in the future.  I'm not
against this in principle, but it is worthwhile to see why sparse is not
handling this case correctly.

Cheers, Andreas

> Could you please confirm this.
> 
> Regards,
> Nayeem
> 
> 

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[nayeem]

On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:

On Sep 12, 2016, at 04:27, Greg KH  wrote:


On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong count 
at exit
l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - unexpected 
unlock

Signed-off-by: Nayeemahmed Badebade 
---
drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)


Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...


The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas



Hi Andreas,

Thank you for your review comments. I did the change according to your 
comments and the diff is attached to mail. But this change doesn't seem 
to fix the sparse warning.
With this change when i compile the code "make C=2 
./drivers/staging/lustre/lustre/", sparse warning still comes:

{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock

  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}

Would it be a good idea to add "__acquires(>l_resource->lr_lock)" 
& "__acquires(>l_lock)" at lock_res_and_lock() and 
"__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" 
at unlock_res_and_lock() ?

Because with that change the sparse warning is fixed.
{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}
Could you please confirm this.

Regards,
Nayeem

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index 1ec4231..2ae463a 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -1293,6 +1293,7 @@ enum lock_res_type {
 
 /** Lock resource. */
 static inline void lock_res(struct ldlm_resource *res)
+			__acquires(>lr_lock)
 {
 	spin_lock(>lr_lock);
 }
@@ -1306,6 +1307,7 @@ static inline void lock_res_nested(struct ldlm_resource *res,
 
 /** Unlock resource. */
 static inline void unlock_res(struct ldlm_resource *res)
+			__releases(>lr_lock)
 {
 	spin_unlock(>lr_lock);
 }
diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..a887d9f 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,7 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+__acquires(>l_lock)
 {
 	spin_lock(>l_lock);
 
@@ -59,6 +60,7 @@ EXPORT_SYMBOL(lock_res_and_lock);
  * Unlock a lock and its resource previously locked with lock_res_and_lock
  */
 void unlock_res_and_lock(struct ldlm_lock *lock)
+		__releases(>l_lock)
 {
 	/* on server-side resource of lock doesn't change */
 	ldlm_clear_res_locked(lock);

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[nayeem]

On Wednesday 14 September 2016 10:44 AM, Dilger, Andreas wrote:

On Sep 12, 2016, at 04:27, Greg KH  wrote:


On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong count 
at exit
l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - unexpected 
unlock

Signed-off-by: Nayeemahmed Badebade 
---
drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)


Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...


The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas



Hi Andreas,

Thank you for your review comments. I did the change according to your 
comments and the diff is attached to mail. But this change doesn't seem 
to fix the sparse warning.
With this change when i compile the code "make C=2 
./drivers/staging/lustre/lustre/", sparse warning still comes:

{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:47:22: 
warning: context imbalance in 'lock_res_and_lock' - wrong count at exit
drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c:62:6: 
warning: context imbalance in 'unlock_res_and_lock' - unexpected unlock

  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}

Would it be a good idea to add "__acquires(>l_resource->lr_lock)" 
& "__acquires(>l_lock)" at lock_res_and_lock() and 
"__releases(>l_resource->lr_lock)" & "__releases(>l_lock)" 
at unlock_res_and_lock() ?

Because with that change the sparse warning is fixed.
{{{
  CHECK   drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.c
  CC [M]  drivers/staging/lustre/lustre/ptlrpc/../../lustre/ldlm/l_lock.o
}}}
Could you please confirm this.

Regards,
Nayeem

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index 1ec4231..2ae463a 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -1293,6 +1293,7 @@ enum lock_res_type {
 
 /** Lock resource. */
 static inline void lock_res(struct ldlm_resource *res)
+			__acquires(>lr_lock)
 {
 	spin_lock(>lr_lock);
 }
@@ -1306,6 +1307,7 @@ static inline void lock_res_nested(struct ldlm_resource *res,
 
 /** Unlock resource. */
 static inline void unlock_res(struct ldlm_resource *res)
+			__releases(>lr_lock)
 {
 	spin_unlock(>lr_lock);
 }
diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..a887d9f 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,7 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+__acquires(>l_lock)
 {
 	spin_lock(>l_lock);
 
@@ -59,6 +60,7 @@ EXPORT_SYMBOL(lock_res_and_lock);
  * Unlock a lock and its resource previously locked with lock_res_and_lock
  */
 void unlock_res_and_lock(struct ldlm_lock *lock)
+		__releases(>l_lock)
 {
 	/* on server-side resource of lock doesn't change */
 	ldlm_clear_res_locked(lock);

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 12, 2016, at 04:27, Greg KH  wrote:
> 
> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
>> Added __acquires / __releases sparse locking annotations
>> to lock_res_and_lock and unlock_res_and_lock functions in
>> l_lock.c, to fix below sparse warnings:
>> 
>> l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
>> count at exit
>> l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
>> unexpected unlock
>> 
>> Signed-off-by: Nayeemahmed Badebade 
>> ---
>> drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
>> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> index ea8840c..c4b9612 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> @@ -45,6 +45,8 @@
>>  * being an atomic operation.
>>  */
>> struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
>> +__acquires(>l_lock)
>> +__acquires(lock->l_resource)
> 
> Hm, these are tricky, I don't want to take this type of change without
> an ack from the lustre developers...

The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Dilger, Andreas]

On Sep 12, 2016, at 04:27, Greg KH  wrote:
> 
> On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
>> Added __acquires / __releases sparse locking annotations
>> to lock_res_and_lock and unlock_res_and_lock functions in
>> l_lock.c, to fix below sparse warnings:
>> 
>> l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
>> count at exit
>> l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
>> unexpected unlock
>> 
>> Signed-off-by: Nayeemahmed Badebade 
>> ---
>> drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
>> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> index ea8840c..c4b9612 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
>> @@ -45,6 +45,8 @@
>>  * being an atomic operation.
>>  */
>> struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
>> +__acquires(>l_lock)
>> +__acquires(lock->l_resource)
> 
> Hm, these are tricky, I don't want to take this type of change without
> an ack from the lustre developers...

The "__acquires(>l_lock)" line here looks correct, along with the
corresponding "__releases(>l_lock)" at unlock_res_and_lock().

The problem, however, is that "l_resource" is not a lock, but rather a
struct.  The call to "lock_res(lock->l_resource)" is actually locking
"lr_lock" internally.

It would be better to add "__acquires(>lr_lock)" at lock_res() and
"__releases(>lr_lock)" at unlock_res().  That will also forestall
any other warnings about an imbalance with lock_res()/unlock_res() or
their callsites.

Cheers, Andreas

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Greg KH]

On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
> Added __acquires / __releases sparse locking annotations
> to lock_res_and_lock and unlock_res_and_lock functions in
> l_lock.c, to fix below sparse warnings:
> 
>  l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
> count at exit
>  l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
> unexpected unlock
> 
> Signed-off-by: Nayeemahmed Badebade 
> ---
>  drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
> index ea8840c..c4b9612 100644
> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
> @@ -45,6 +45,8 @@
>   * being an atomic operation.
>   */
>  struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
> + __acquires(>l_lock)
> + __acquires(lock->l_resource)

Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...

thanks,

greg k-h

Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Greg KH]

On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote:
> Added __acquires / __releases sparse locking annotations
> to lock_res_and_lock and unlock_res_and_lock functions in
> l_lock.c, to fix below sparse warnings:
> 
>  l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
> count at exit
>  l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
> unexpected unlock
> 
> Signed-off-by: Nayeemahmed Badebade 
> ---
>  drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
> b/drivers/staging/lustre/lustre/ldlm/l_lock.c
> index ea8840c..c4b9612 100644
> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
> @@ -45,6 +45,8 @@
>   * being an atomic operation.
>   */
>  struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
> + __acquires(>l_lock)
> + __acquires(lock->l_resource)

Hm, these are tricky, I don't want to take this type of change without
an ack from the lustre developers...

thanks,

greg k-h

[PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Nayeemahmed Badebade]

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

 l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
count at exit
 l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
unexpected unlock

Signed-off-by: Nayeemahmed Badebade 
---
 drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)
 {
spin_lock(>l_lock);

@@ -59,6 +61,8 @@ EXPORT_SYMBOL(lock_res_and_lock);
  * Unlock a lock and its resource previously locked with lock_res_and_lock
  */
 void unlock_res_and_lock(struct ldlm_lock *lock)
+   __releases(lock->l_resource)
+   __releases(>l_lock)
 {
/* on server-side resource of lock doesn't change */
ldlm_clear_res_locked(lock);
--
1.9.1

[PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings

[Nayeemahmed Badebade]

Added __acquires / __releases sparse locking annotations
to lock_res_and_lock and unlock_res_and_lock functions in
l_lock.c, to fix below sparse warnings:

 l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong 
count at exit
 l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - 
unexpected unlock

Signed-off-by: Nayeemahmed Badebade 
---
 drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c 
b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index ea8840c..c4b9612 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,6 +45,8 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
+   __acquires(>l_lock)
+   __acquires(lock->l_resource)
 {
spin_lock(>l_lock);

@@ -59,6 +61,8 @@ EXPORT_SYMBOL(lock_res_and_lock);
  * Unlock a lock and its resource previously locked with lock_res_and_lock
  */
 void unlock_res_and_lock(struct ldlm_lock *lock)
+   __releases(lock->l_resource)
+   __releases(>l_lock)
 {
/* on server-side resource of lock doesn't change */
ldlm_clear_res_locked(lock);
--
1.9.1