Re: [PATCH 14/17] btrfs: introduce BTRFS_NESTING_NEW_ROOT for adding new roots

David Sterba
2020-08-14T14:46:14Z

On Mon, Aug 10, 2020 at 11:42:39AM -0400, Josef Bacik wrote:
> --- a/fs/btrfs/locking.h
> +++ b/fs/btrfs/locking.h
> @@ -16,6 +16,11 @@
> #define BTRFS_WRITE_LOCK_BLOCKING 3
> #define BTRFS_READ_LOCK_BLOCKING 4
>
> +/*
> + * We are limited in number of subclasses by MAX_LOCKDEP_SUBCLASSES, which at
> + * the time of this patch is 8, which is how many we're using here. Keep this
> + * in mind if you decide you want to add another subclass.
> + */
> enum btrfs_lock_nesting {
> BTRFS_NESTING_NORMAL = 0,
>
> @@ -55,6 +60,15 @@ enum btrfs_lock_nesting {
> * handle this case where we need to allocate a new split block.
> */
> BTRFS_NESTING_SPLIT,
> +
> + /*
> + * When promoting a new block to a root we need to have a special
> + * subclass so we don't confuse lockdep, as it will appear that we are
> + * locking a higher level node before a lower level one. Copying also
> + * has this problem as it appears we're locking the same block again
> + * when we make a snapshot of an existing root.
> + */
> + BTRFS_NESTING_NEW_ROOT,
> };

To prevent accidental addition (and not reading the comment about the
maximum count), I suggest to use the static_assert with one extra
definition like

--- a/fs/btrfs/locking.h
+++ b/fs/btrfs/locking.h
@@ -14,11 +14,6 @@
#define BTRFS_WRITE_LOCK 1
#define BTRFS_READ_LOCK 2

-/*
- * We are limited in number of subclasses by MAX_LOCKDEP_SUBCLASSES, which at
- * the time of this patch is 8, which is how many we're using here. Keep this
- * in mind if you decide you want to add another subclass.
- */
enum btrfs_lock_nesting {
BTRFS_NESTING_NORMAL = 0,

@@ -67,8 +62,19 @@ enum btrfs_lock_nesting {
* when we make a snapshot of an existing root.
*/
BTRFS_NESTING_NEW_ROOT,
+
+ /*
+ * We are limited in number of subclasses by MAX_LOCKDEP_SUBCLASSES,
+ * which at the time of this patch is 8, which is how many we're using
+ * here. Keep this in mind if you decide you want to add another
+ * subclass.
+ */
+ BTRFS_NESTING_MAX
};

+static_assert(BTRFS_NESTING_MAX <= MAX_LOCKDEP_SUBCLASSES,
+ "too many lock subclasses defined");
+
struct btrfs_path;

void __btrfs_tree_lock(struct extent_buffer *eb, enum btrfs_lock_nesting nest);
---