Skip to content

Commit

Permalink
btrfs: move extent_buffer::lock_owner to debug section
Browse files Browse the repository at this point in the history
The lock_owner is used for a rare corruption case and we haven't seen
any reports in years. Move it to the debugging section of eb.  To close
the holes also move log_index so the final layout looks like:

struct extent_buffer {
        u64                        start;                /*     0     8 */
        long unsigned int          len;                  /*     8     8 */
        long unsigned int          bflags;               /*    16     8 */
        struct btrfs_fs_info *     fs_info;              /*    24     8 */
        spinlock_t                 refs_lock;            /*    32     4 */
        atomic_t                   refs;                 /*    36     4 */
        int                        read_mirror;          /*    40     4 */
        s8                         log_index;            /*    44     1 */

        /* XXX 3 bytes hole, try to pack */

        struct callback_head       callback_head __attribute__((__aligned__(8))); /*    48    16 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        struct rw_semaphore        lock;                 /*    64    40 */
        struct page *              pages[16];            /*   104   128 */

        /* size: 232, cachelines: 4, members: 11 */
        /* sum members: 229, holes: 1, sum holes: 3 */
        /* forced alignments: 1, forced holes: 1, sum forced holes: 3 */
        /* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

This saves 8 bytes in total and still keeps the lock on a separate cacheline.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
  • Loading branch information
David Sterba committed Oct 12, 2023
1 parent 321f499 commit 150cce2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 14 deletions.
32 changes: 23 additions & 9 deletions fs/btrfs/extent-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -4803,6 +4803,28 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
return ret;
}

#ifdef CONFIG_BTRFS_DEBUG
/*
* Extra safety check in case the extent tree is corrupted and extent allocator
* chooses to use a tree block which is already used and locked.
*/
static bool check_eb_lock_owner(const struct extent_buffer *eb)
{
if (eb->lock_owner == current->pid) {
btrfs_err_rl(eb->fs_info,
"tree block %llu owner %llu already locked by pid=%d, extent tree corruption detected",
eb->start, btrfs_header_owner(eb), current->pid);
return true;
}
return false;
}
#else
static bool check_eb_lock_owner(struct extent_buffer *eb)
{
return false;
}
#endif

static struct extent_buffer *
btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
u64 bytenr, int level, u64 owner,
Expand All @@ -4816,15 +4838,7 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
if (IS_ERR(buf))
return buf;

/*
* Extra safety check in case the extent tree is corrupted and extent
* allocator chooses to use a tree block which is already used and
* locked.
*/
if (buf->lock_owner == current->pid) {
btrfs_err_rl(fs_info,
"tree block %llu owner %llu already locked by pid=%d, extent tree corruption detected",
buf->start, btrfs_header_owner(buf), current->pid);
if (check_eb_lock_owner(buf)) {
free_extent_buffer(buf);
return ERR_PTR(-EUCLEAN);
}
Expand Down
4 changes: 2 additions & 2 deletions fs/btrfs/extent_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ struct extent_buffer {
spinlock_t refs_lock;
atomic_t refs;
int read_mirror;
struct rcu_head rcu_head;
pid_t lock_owner;
/* >= 0 if eb belongs to a log tree, -1 otherwise */
s8 log_index;
struct rcu_head rcu_head;

struct rw_semaphore lock;

struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
#ifdef CONFIG_BTRFS_DEBUG
struct list_head leak_list;
pid_t lock_owner;
#endif
};

Expand Down
15 changes: 12 additions & 3 deletions fs/btrfs/locking.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ void btrfs_maybe_reset_lockdep_class(struct btrfs_root *root, struct extent_buff

#endif

#ifdef CONFIG_BTRFS_DEBUG
static void btrfs_set_eb_lock_owner(struct extent_buffer *eb, pid_t owner)
{
eb->lock_owner = owner;
}
#else
static void btrfs_set_eb_lock_owner(struct extent_buffer *eb, pid_t owner) { }
#endif

/*
* Extent buffer locking
* =====================
Expand Down Expand Up @@ -165,7 +174,7 @@ int btrfs_try_tree_read_lock(struct extent_buffer *eb)
int btrfs_try_tree_write_lock(struct extent_buffer *eb)
{
if (down_write_trylock(&eb->lock)) {
eb->lock_owner = current->pid;
btrfs_set_eb_lock_owner(eb, current->pid);
trace_btrfs_try_tree_write_lock(eb);
return 1;
}
Expand Down Expand Up @@ -198,7 +207,7 @@ void __btrfs_tree_lock(struct extent_buffer *eb, enum btrfs_lock_nesting nest)
start_ns = ktime_get_ns();

down_write_nested(&eb->lock, nest);
eb->lock_owner = current->pid;
btrfs_set_eb_lock_owner(eb, current->pid);
trace_btrfs_tree_lock(eb, start_ns);
}

Expand All @@ -213,7 +222,7 @@ void btrfs_tree_lock(struct extent_buffer *eb)
void btrfs_tree_unlock(struct extent_buffer *eb)
{
trace_btrfs_tree_unlock(eb);
eb->lock_owner = 0;
btrfs_set_eb_lock_owner(eb, 0);
up_write(&eb->lock);
}

Expand Down

0 comments on commit 150cce2

Please sign in to comment.