[PATCH] Btrfs: remove BUG_ON from get_restripe_target

[PATCH] Btrfs: remove BUG_ON from get_restripe_target

[Bobby Powers]

spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
to fail the mount.  There is documentation pending as to why checking
for spin_is_locked is a bad idea:

https://lkml.org/lkml/2012/3/27/413

As this was the only location in fs/btrfs/extent-tree.c that did
lock-correctness checking in a BUG_ON, simply remove it.

Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
CC: Ilya Dryomov <idryomov@gmail.com>
CC: Chris Mason <chris.mason@oracle.com>
CC: Andi Kleen <ak@linux.intel.com>
CC: linux-btrfs@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 fs/btrfs/extent-tree.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a844204..c98b073 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3158,9 +3158,6 @@ static u64 get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
 	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
 	u64 target = 0;
 
-	BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) &&
-	       !spin_is_locked(&fs_info->balance_lock));
-
 	if (!bctl)
 		return 0;
 
-- 
1.7.10.rc3.3.g19a6c

Re: [PATCH] Btrfs: remove BUG_ON from get_restripe_target

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/04/2012 04:19 PM, Bobby Powers wrote:
> spin_is_locked always returns 0 on non-SMP systems, which causes 
> btrfs to fail the mount.  There is documentation pending as to why 
> checking for spin_is_locked is a bad idea:
> 
> https://lkml.org/lkml/2012/3/27/413
> 
> As this was the only location in fs/btrfs/extent-tree.c that did 
> lock-correctness checking in a BUG_ON, simply remove it.
> 
> Signed-off-by: Bobby Powers <bobbypowers@gmail.com> CC: Ilya 
> Dryomov <idryomov@gmail.com> CC: Chris Mason 
> <chris.mason@oracle.com> CC: Andi Kleen <ak@linux.intel.com> CC: 
> linux-btrfs@vger.kernel.org CC: linux-kernel@vger.kernel.org --- 
> fs/btrfs/extent-tree.c |    3 --- 1 file changed, 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 
> a844204..c98b073 100644 --- a/fs/btrfs/extent-tree.c +++ 
> b/fs/btrfs/extent-tree.c @@ -3158,9 +3158,6 @@ static u64 
> get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags) 
> struct btrfs_balance_control *bctl = fs_info->balance_ctl; u64 
> target = 0;
> 
> -	BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) && - 
> !spin_is_locked(&fs_info->balance_lock)); - if (!bctl) return 0;
> 

Why not replace both of these with lockdep_assert_held as Andi
suggested in his doc?

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPfOtVAAoJEB57S2MheeWyew4P/ju2FiKPWvFEhShP2BQzmPPL
wYoYP9cEmuoJbr1FXnpVXRy3RJsS+Dv9IHyUt2l8WRddO5PeTC08fS3a1PsR6Lky
dnDBpOTsvBhdUPClA+9cu9HtNZ488PKALTgNX7kocVYm+vd0vkE54Iv0OWRvIMEA
5nmm/r2MJwgQmTsFwAWojxfiSEJNzRSJ6GXZFbIfwGOaJIx7MmnPg4R3PKU1SZiF
ogKIwocfsTA/T7eplK58+EqtQnfTGTKzAbaEQvX/w1ryRRXHqD2zdk2p+zSAtpN6
swnhu156Bb9t2EUPTv9KiYth0BoYhYy9ppdp2Wyh0hX3lYCAP1SrwBVIYCqpvdqr
CuipFWmqbupW41IjQc6bjoIwaVlGNsqwDY3NNrjR+kb29k3+/3MT8QH1ZXLhlpTB
cUixluE62J8QllW4u3Wa2mLMTqdolcWCCTdh4yUqE+8jxguXUKhoTni1vwApmkru
PM158CLPdxWCnDe1TaGcRYcBoweWPl6UDaVj8W+LdSVcYsycZhwehvDg2amX6pdg
9QFUf25PbDzEVw99w3f2hMhRG5pERRheLqPcFUVbnqZYkZBACt9XtIYBINlREoYW
ACjr9dJszluF9dEKWOmlKhsah3gAGJJoC5+QU8oR+vpxKdrI+8vQ+NYtVhCr0hVA
CO/KEEcwNaobsCWiAbSr
=5Op/
-----END PGP SIGNATURE-----

Re: [PATCH] Btrfs: remove BUG_ON from get_restripe_target

[Bobby Powers]

On Wed, Apr 4, 2012 at 8:46 PM, Jeff Mahoney <jeffm@suse.de> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/04/2012 04:19 PM, Bobby Powers wrote:
>> spin_is_locked always returns 0 on non-SMP systems, which causes
>> btrfs to fail the mount.  There is documentation pending as to why
>> checking for spin_is_locked is a bad idea:
>>
>> https://lkml.org/lkml/2012/3/27/413
>>
>> As this was the only location in fs/btrfs/extent-tree.c that did
>> lock-correctness checking in a BUG_ON, simply remove it.
>>
>> Signed-off-by: Bobby Powers <bobbypowers@gmail.com> CC: Ilya
>> Dryomov <idryomov@gmail.com> CC: Chris Mason
>> <chris.mason@oracle.com> CC: Andi Kleen <ak@linux.intel.com> CC:
>> linux-btrfs@vger.kernel.org CC: linux-kernel@vger.kernel.org ---
>> fs/btrfs/extent-tree.c |    3 --- 1 file changed, 3 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index
>> a844204..c98b073 100644 --- a/fs/btrfs/extent-tree.c +++
>> b/fs/btrfs/extent-tree.c @@ -3158,9 +3158,6 @@ static u64
>> get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
>> struct btrfs_balance_control *bctl = fs_info->balance_ctl; u64
>> target = 0;
>>
>> -     BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) && -
>> !spin_is_locked(&fs_info->balance_lock)); - if (!bctl) return 0;
>>
>
> Why not replace both of these with lockdep_assert_held as Andi
> suggested in his doc?

The complication here is that the existing statement was asserting
that _either_ volume_mutex was held _or_ balance_lock was held -
lockdep_assert_held is defined as:

#define lockdep_assert_held(l)	WARN_ON(debug_locks && !lockdep_is_held(l))

which doesn't map to the existing logic.  Although I could be missing something.

Sorry for the double email, forgot to turn off html mail initially.

yours,
Bobby

> - -Jeff
>
> - --
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.18 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQIcBAEBAgAGBQJPfOtVAAoJEB57S2MheeWyew4P/ju2FiKPWvFEhShP2BQzmPPL
> wYoYP9cEmuoJbr1FXnpVXRy3RJsS+Dv9IHyUt2l8WRddO5PeTC08fS3a1PsR6Lky
> dnDBpOTsvBhdUPClA+9cu9HtNZ488PKALTgNX7kocVYm+vd0vkE54Iv0OWRvIMEA
> 5nmm/r2MJwgQmTsFwAWojxfiSEJNzRSJ6GXZFbIfwGOaJIx7MmnPg4R3PKU1SZiF
> ogKIwocfsTA/T7eplK58+EqtQnfTGTKzAbaEQvX/w1ryRRXHqD2zdk2p+zSAtpN6
> swnhu156Bb9t2EUPTv9KiYth0BoYhYy9ppdp2Wyh0hX3lYCAP1SrwBVIYCqpvdqr
> CuipFWmqbupW41IjQc6bjoIwaVlGNsqwDY3NNrjR+kb29k3+/3MT8QH1ZXLhlpTB
> cUixluE62J8QllW4u3Wa2mLMTqdolcWCCTdh4yUqE+8jxguXUKhoTni1vwApmkru
> PM158CLPdxWCnDe1TaGcRYcBoweWPl6UDaVj8W+LdSVcYsycZhwehvDg2amX6pdg
> 9QFUf25PbDzEVw99w3f2hMhRG5pERRheLqPcFUVbnqZYkZBACt9XtIYBINlREoYW
> ACjr9dJszluF9dEKWOmlKhsah3gAGJJoC5+QU8oR+vpxKdrI+8vQ+NYtVhCr0hVA
> CO/KEEcwNaobsCWiAbSr
> =5Op/
> -----END PGP SIGNATURE-----

Re: [PATCH] Btrfs: remove BUG_ON from get_restripe_target

[Bobby Powers]

On Wed, Apr 4, 2012 at 9:19 PM, Bobby Powers <bobbypowers@gmail.com> wrote:
> On Wed, Apr 4, 2012 at 8:46 PM, Jeff Mahoney <jeffm@suse.de> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 04/04/2012 04:19 PM, Bobby Powers wrote:
>>> spin_is_locked always returns 0 on non-SMP systems, which causes
>>> btrfs to fail the mount.  There is documentation pending as to why
>>> checking for spin_is_locked is a bad idea:
>>>
>>> https://lkml.org/lkml/2012/3/27/413
>>>
>>> As this was the only location in fs/btrfs/extent-tree.c that did
>>> lock-correctness checking in a BUG_ON, simply remove it.
>>>
>>> Signed-off-by: Bobby Powers <bobbypowers@gmail.com> CC: Ilya
>>> Dryomov <idryomov@gmail.com> CC: Chris Mason
>>> <chris.mason@oracle.com> CC: Andi Kleen <ak@linux.intel.com> CC:
>>> linux-btrfs@vger.kernel.org CC: linux-kernel@vger.kernel.org ---
>>> fs/btrfs/extent-tree.c |    3 --- 1 file changed, 3 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index
>>> a844204..c98b073 100644 --- a/fs/btrfs/extent-tree.c +++
>>> b/fs/btrfs/extent-tree.c @@ -3158,9 +3158,6 @@ static u64
>>> get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
>>> struct btrfs_balance_control *bctl = fs_info->balance_ctl; u64
>>> target = 0;
>>>
>>> -     BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) && -
>>> !spin_is_locked(&fs_info->balance_lock)); - if (!bctl) return 0;
>>>
>>
>> Why not replace both of these with lockdep_assert_held as Andi
>> suggested in his doc?
>
> The complication here is that the existing statement was asserting
> that _either_ volume_mutex was held _or_ balance_lock was held -
> lockdep_assert_held is defined as:
>
> #define lockdep_assert_held(l)  WARN_ON(debug_locks && !lockdep_is_held(l))

Well, I guess it works fine if lockdep.h defines lockdep_is_held(l)
for the !LOCKDEP case:

       BUG_ON(debug_locks && !lockdep_is_held(&fs_info->volume_mutex) &&
              !lockdep_is_held(&fs_info->balance_lock));

> which doesn't map to the existing logic.  Although I could be missing something.
>
> Sorry for the double email, forgot to turn off html mail initially.
>
> yours,
> Bobby
>
>> - -Jeff
>>
>> - --
>> Jeff Mahoney
>> SUSE Labs
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v2.0.18 (GNU/Linux)
>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>>
>> iQIcBAEBAgAGBQJPfOtVAAoJEB57S2MheeWyew4P/ju2FiKPWvFEhShP2BQzmPPL
>> wYoYP9cEmuoJbr1FXnpVXRy3RJsS+Dv9IHyUt2l8WRddO5PeTC08fS3a1PsR6Lky
>> dnDBpOTsvBhdUPClA+9cu9HtNZ488PKALTgNX7kocVYm+vd0vkE54Iv0OWRvIMEA
>> 5nmm/r2MJwgQmTsFwAWojxfiSEJNzRSJ6GXZFbIfwGOaJIx7MmnPg4R3PKU1SZiF
>> ogKIwocfsTA/T7eplK58+EqtQnfTGTKzAbaEQvX/w1ryRRXHqD2zdk2p+zSAtpN6
>> swnhu156Bb9t2EUPTv9KiYth0BoYhYy9ppdp2Wyh0hX3lYCAP1SrwBVIYCqpvdqr
>> CuipFWmqbupW41IjQc6bjoIwaVlGNsqwDY3NNrjR+kb29k3+/3MT8QH1ZXLhlpTB
>> cUixluE62J8QllW4u3Wa2mLMTqdolcWCCTdh4yUqE+8jxguXUKhoTni1vwApmkru
>> PM158CLPdxWCnDe1TaGcRYcBoweWPl6UDaVj8W+LdSVcYsycZhwehvDg2amX6pdg
>> 9QFUf25PbDzEVw99w3f2hMhRG5pERRheLqPcFUVbnqZYkZBACt9XtIYBINlREoYW
>> ACjr9dJszluF9dEKWOmlKhsah3gAGJJoC5+QU8oR+vpxKdrI+8vQ+NYtVhCr0hVA
>> CO/KEEcwNaobsCWiAbSr
>> =5Op/
>> -----END PGP SIGNATURE-----

[PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Bobby Powers]

spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
to fail the mount.  There is documentation pending as to why checking
for spin_is_locked is a bad idea:

https://lkml.org/lkml/2012/3/27/413

The suggested lockdep_assert_held() is not appropriate in this case,
as what get_restripe_target() is checking for is that either
volume_mutex is held or balance_lock is held.  Luckily
lockdep_assert_held() is a simple macro:

WARN_ON(debug_locks && !lockdep_is_held(l))

We can mimic the structure in get_restripe_target(), but we need to
make sure lockdep_is_held() is defined for the !LOCKDEP case.

CC: Ilya Dryomov <idryomov@gmail.com>
CC: Chris Mason <chris.mason@oracle.com>
CC: Andi Kleen <ak@linux.intel.com>
CC: Jeff Mahoney <jeffm@suse.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: linux-kernel@vger.kernel.org
Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
---
 fs/btrfs/extent-tree.c  |    5 +++--
 include/linux/lockdep.h |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a844204..4d13eb1 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -24,6 +24,7 @@
 #include <linux/kthread.h>
 #include <linux/slab.h>
 #include <linux/ratelimit.h>
+#include <linux/lockdep.h>
 #include "compat.h"
 #include "hash.h"
 #include "ctree.h"
@@ -3158,8 +3159,8 @@ static u64 get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
 	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
 	u64 target = 0;
 
-	BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) &&
-	       !spin_is_locked(&fs_info->balance_lock));
+	BUG_ON(debug_locks && !lockdep_is_held(&fs_info->volume_mutex) &&
+	       !lockdep_is_held(&fs_info->balance_lock));
 
 	if (!bctl)
 		return 0;
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index d36619e..94c0edb 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -392,6 +392,7 @@ struct lock_class_key { };
 
 #define lockdep_depth(tsk)	(0)
 
+#define lockdep_is_held(l)	(0)
 #define lockdep_assert_held(l)			do { } while (0)
 
 #define lockdep_recursing(tsk)			(0)
-- 
1.7.10.rc3.3.g19a6c

Re: [PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Bobby Powers]

On Wed, Apr 4, 2012 at 10:04 PM, Bobby Powers <bobbypowers@gmail.com> wrote:
> spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
> to fail the mount.  There is documentation pending as to why checking

I guess I should be explicit in stating that this is a regression, so
this patch or something else that addresses it should be pulled into
3.4.

> for spin_is_locked is a bad idea:
>
> https://lkml.org/lkml/2012/3/27/413
>
> The suggested lockdep_assert_held() is not appropriate in this case,
> as what get_restripe_target() is checking for is that either
> volume_mutex is held or balance_lock is held.  Luckily
> lockdep_assert_held() is a simple macro:
>
> WARN_ON(debug_locks && !lockdep_is_held(l))
>
> We can mimic the structure in get_restripe_target(), but we need to
> make sure lockdep_is_held() is defined for the !LOCKDEP case.
>
> CC: Ilya Dryomov <idryomov@gmail.com>
> CC: Chris Mason <chris.mason@oracle.com>
> CC: Andi Kleen <ak@linux.intel.com>
> CC: Jeff Mahoney <jeffm@suse.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
> ---
>  fs/btrfs/extent-tree.c  |    5 +++--
>  include/linux/lockdep.h |    1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a844204..4d13eb1 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -24,6 +24,7 @@
>  #include <linux/kthread.h>
>  #include <linux/slab.h>
>  #include <linux/ratelimit.h>
> +#include <linux/lockdep.h>
>  #include "compat.h"
>  #include "hash.h"
>  #include "ctree.h"
> @@ -3158,8 +3159,8 @@ static u64 get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
>        struct btrfs_balance_control *bctl = fs_info->balance_ctl;
>        u64 target = 0;
>
> -       BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) &&
> -              !spin_is_locked(&fs_info->balance_lock));
> +       BUG_ON(debug_locks && !lockdep_is_held(&fs_info->volume_mutex) &&
> +              !lockdep_is_held(&fs_info->balance_lock));
>
>        if (!bctl)
>                return 0;
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index d36619e..94c0edb 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -392,6 +392,7 @@ struct lock_class_key { };
>
>  #define lockdep_depth(tsk)     (0)
>
> +#define lockdep_is_held(l)     (0)
>  #define lockdep_assert_held(l)                 do { } while (0)
>
>  #define lockdep_recursing(tsk)                 (0)
> --
> 1.7.10.rc3.3.g19a6c
>

Re: [PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Ilya Dryomov]

On Thu, Apr 05, 2012 at 12:23:01PM -0400, Bobby Powers wrote:
> On Wed, Apr 4, 2012 at 10:04 PM, Bobby Powers <bobbypowers@gmail.com> wrote:
> > spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
> > to fail the mount.  There is documentation pending as to why checking
> 
> I guess I should be explicit in stating that this is a regression, so
> this patch or something else that addresses it should be pulled into
> 3.4.

Yes, this is a regression and spin_is_locked() definitely has to go.  I
don't have a strong opinion on this assert, if there are objections to
v2 I'm OK with ripping that BUG_ON entirely and replacing it with a
comment (this function and its callers are WIP).

Thanks,

		Ilya

Re: [PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Mitch Harder]

On Thu, Apr 5, 2012 at 11:51 AM, Ilya Dryomov <idryomov@gmail.com> wrote:
> On Thu, Apr 05, 2012 at 12:23:01PM -0400, Bobby Powers wrote:
>> On Wed, Apr 4, 2012 at 10:04 PM, Bobby Powers <bobbypowers@gmail.com> wrote:
>> > spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
>> > to fail the mount.  There is documentation pending as to why checking
>>
>> I guess I should be explicit in stating that this is a regression, so
>> this patch or something else that addresses it should be pulled into
>> 3.4.
>
> Yes, this is a regression and spin_is_locked() definitely has to go.  I
> don't have a strong opinion on this assert, if there are objections to
> v2 I'm OK with ripping that BUG_ON entirely and replacing it with a
> comment (this function and its callers are WIP).
>

I'm still hitting this BUG_ON after applying this patch on my single
CPU AthlonXP x86 system.

I'm running a 3.3.1 kernel merged with the for-linus branch, and had
this patch applied.

I am currently testing with the BUG_ON commented out.  The btrfs
partitions mount without error, and I haven't encountered any
immediate issues.

Re: [PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Ilya Dryomov]

On Wed, Apr 04, 2012 at 10:04:12PM -0400, Bobby Powers wrote:
> spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
> to fail the mount.  There is documentation pending as to why checking
> for spin_is_locked is a bad idea:
> 
> https://lkml.org/lkml/2012/3/27/413
> 
> The suggested lockdep_assert_held() is not appropriate in this case,
> as what get_restripe_target() is checking for is that either
> volume_mutex is held or balance_lock is held.  Luckily
> lockdep_assert_held() is a simple macro:
> 
> WARN_ON(debug_locks && !lockdep_is_held(l))
> 
> We can mimic the structure in get_restripe_target(), but we need to
> make sure lockdep_is_held() is defined for the !LOCKDEP case.
> 
> CC: Ilya Dryomov <idryomov@gmail.com>
> CC: Chris Mason <chris.mason@oracle.com>
> CC: Andi Kleen <ak@linux.intel.com>
> CC: Jeff Mahoney <jeffm@suse.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
> ---
>  fs/btrfs/extent-tree.c  |    5 +++--
>  include/linux/lockdep.h |    1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a844204..4d13eb1 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -24,6 +24,7 @@
>  #include <linux/kthread.h>
>  #include <linux/slab.h>
>  #include <linux/ratelimit.h>
> +#include <linux/lockdep.h>
>  #include "compat.h"
>  #include "hash.h"
>  #include "ctree.h"
> @@ -3158,8 +3159,8 @@ static u64 get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
>  	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
>  	u64 target = 0;
>  
> -	BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) &&
> -	       !spin_is_locked(&fs_info->balance_lock));
> +	BUG_ON(debug_locks && !lockdep_is_held(&fs_info->volume_mutex) &&
> +	       !lockdep_is_held(&fs_info->balance_lock));
>  
>  	if (!bctl)
>  		return 0;
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index d36619e..94c0edb 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -392,6 +392,7 @@ struct lock_class_key { };
>  
>  #define lockdep_depth(tsk)	(0)
>  
> +#define lockdep_is_held(l)	(0)
>  #define lockdep_assert_held(l)			do { } while (0)
>  
>  #define lockdep_recursing(tsk)			(0)
> -- 
> 1.7.10.rc3.3.g19a6c

OK, Mitch's report prompted me to actually take a look.  This patch is
wrong: by defining lockdep_is_held(l) to 0 in !LOCKDEP case you
effectively mimic the behaviour of spin_is_locked() which is what
getting us in the first place.

get_restripe_target() interface is WIP so I will replace BUG_ON with a
comment and send a patch through btrfs tree.

Thanks,

		Ilya

Re: [PATCH v2] Btrfs, lockdep: get_restripe_target: use lockdep in BUG_ON

[Bobby Powers]

On Fri, Apr 6, 2012 at 4:05 PM, Ilya Dryomov <idryomov@gmail.com> wrote:
> On Wed, Apr 04, 2012 at 10:04:12PM -0400, Bobby Powers wrote:
>> spin_is_locked always returns 0 on non-SMP systems, which causes btrfs
>> to fail the mount.  There is documentation pending as to why checking
>> for spin_is_locked is a bad idea:
>>
>> https://lkml.org/lkml/2012/3/27/413
>>
>> The suggested lockdep_assert_held() is not appropriate in this case,
>> as what get_restripe_target() is checking for is that either
>> volume_mutex is held or balance_lock is held.  Luckily
>> lockdep_assert_held() is a simple macro:
>>
>> WARN_ON(debug_locks && !lockdep_is_held(l))
>>
>> We can mimic the structure in get_restripe_target(), but we need to
>> make sure lockdep_is_held() is defined for the !LOCKDEP case.
>>
>> CC: Ilya Dryomov <idryomov@gmail.com>
>> CC: Chris Mason <chris.mason@oracle.com>
>> CC: Andi Kleen <ak@linux.intel.com>
>> CC: Jeff Mahoney <jeffm@suse.de>
>> CC: Ingo Molnar <mingo@redhat.com>
>> CC: linux-kernel@vger.kernel.org
>> Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
>> ---
>>  fs/btrfs/extent-tree.c  |    5 +++--
>>  include/linux/lockdep.h |    1 +
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index a844204..4d13eb1 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/kthread.h>
>>  #include <linux/slab.h>
>>  #include <linux/ratelimit.h>
>> +#include <linux/lockdep.h>
>>  #include "compat.h"
>>  #include "hash.h"
>>  #include "ctree.h"
>> @@ -3158,8 +3159,8 @@ static u64 get_restripe_target(struct btrfs_fs_info *fs_info, u64 flags)
>>       struct btrfs_balance_control *bctl = fs_info->balance_ctl;
>>       u64 target = 0;
>>
>> -     BUG_ON(!mutex_is_locked(&fs_info->volume_mutex) &&
>> -            !spin_is_locked(&fs_info->balance_lock));
>> +     BUG_ON(debug_locks && !lockdep_is_held(&fs_info->volume_mutex) &&
>> +            !lockdep_is_held(&fs_info->balance_lock));
>>
>>       if (!bctl)
>>               return 0;
>> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
>> index d36619e..94c0edb 100644
>> --- a/include/linux/lockdep.h
>> +++ b/include/linux/lockdep.h
>> @@ -392,6 +392,7 @@ struct lock_class_key { };
>>
>>  #define lockdep_depth(tsk)   (0)
>>
>> +#define lockdep_is_held(l)   (0)
>>  #define lockdep_assert_held(l)                       do { } while (0)
>>
>>  #define lockdep_recursing(tsk)                       (0)
>> --
>> 1.7.10.rc3.3.g19a6c
>
> OK, Mitch's report prompted me to actually take a look.  This patch is
> wrong: by defining lockdep_is_held(l) to 0 in !LOCKDEP case you
> effectively mimic the behaviour of spin_is_locked() which is what
> getting us in the first place.
>
> get_restripe_target() interface is WIP so I will replace BUG_ON with a
> comment and send a patch through btrfs tree.

Hah, good point...

> Thanks,
>
>                Ilya