Skip to content

Commit 1b6e068

Browse files
fdmananakdave
authored andcommitted
btrfs: add and use helper to verify the calling task has locked the inode
We have a few places that check if we have the inode locked by doing: ASSERT(inode_is_locked(vfs_inode)); This actually proved to be useful several times as if assertions are enabled (and by default they are in many distros) it immediately triggers a crash which is impossible for users to miss. However that doesn't check if the lock is held by the calling task, so the check passes if some other task locked the inode. Using one of the lockdep functions to check the lock is held, like lockdep_assert_held() for example, does check that the calling task holds the lock, and if that's not the case it produces a warning and stack trace in dmesg. However, despite the misleading "assert" in the name of the lockdep helpers, it does not trigger a crash/BUG_ON(), just a warning and splat in dmesg, which is easy to get unnoticed by users who may have lockdep enabled. So add a helper that does the ASSERT() and calls lockdep_assert_held() immediately after and use it every where we check the inode is locked. Like this if the lock is held by some other task we get the warning in dmesg which is caught by fstests, very helpful during development, and may also be occassionaly noticed by users with lockdep enabled. Reviewed-by: Josef Bacik <[email protected]> Signed-off-by: Filipe Manana <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 3368597 commit 1b6e068

File tree

6 files changed

+15
-7
lines changed

6 files changed

+15
-7
lines changed

fs/btrfs/btrfs_inode.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,14 @@ static inline bool btrfs_inode_can_compress(const struct btrfs_inode *inode)
505505
return true;
506506
}
507507

508+
static inline void btrfs_assert_inode_locked(struct btrfs_inode *inode)
509+
{
510+
/* Immediately trigger a crash if the inode is not locked. */
511+
ASSERT(inode_is_locked(&inode->vfs_inode));
512+
/* Trigger a splat in dmesg if this task is not holding the lock. */
513+
lockdep_assert_held(&inode->vfs_inode.i_rwsem);
514+
}
515+
508516
/* Array of bytes with variable length, hexadecimal format 0x1234 */
509517
#define CSUM_FMT "0x%*phN"
510518
#define CSUM_FMT_VALUE(size, bytes) size, bytes

fs/btrfs/file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1617,7 +1617,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
16171617
if (current->journal_info == BTRFS_TRANS_DIO_WRITE_STUB) {
16181618
skip_ilock = true;
16191619
current->journal_info = NULL;
1620-
lockdep_assert_held(&inode->vfs_inode.i_rwsem);
1620+
btrfs_assert_inode_locked(inode);
16211621
}
16221622

16231623
trace_btrfs_sync_file(file, datasync);

fs/btrfs/ordered-data.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode,
10151015
{
10161016
struct rb_node *n;
10171017

1018-
ASSERT(inode_is_locked(&inode->vfs_inode));
1018+
btrfs_assert_inode_locked(inode);
10191019

10201020
spin_lock_irq(&inode->ordered_tree_lock);
10211021
for (n = rb_first(&inode->ordered_tree); n; n = rb_next(n)) {

fs/btrfs/tree-log.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2877,7 +2877,7 @@ void btrfs_release_log_ctx_extents(struct btrfs_log_ctx *ctx)
28772877
struct btrfs_ordered_extent *ordered;
28782878
struct btrfs_ordered_extent *tmp;
28792879

2880-
ASSERT(inode_is_locked(&ctx->inode->vfs_inode));
2880+
btrfs_assert_inode_locked(ctx->inode);
28812881

28822882
list_for_each_entry_safe(ordered, tmp, &ctx->ordered_extents, log_list) {
28832883
list_del_init(&ordered->log_list);

fs/btrfs/verity.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ static int rollback_verity(struct btrfs_inode *inode)
460460
struct btrfs_root *root = inode->root;
461461
int ret;
462462

463-
ASSERT(inode_is_locked(&inode->vfs_inode));
463+
btrfs_assert_inode_locked(inode);
464464
truncate_inode_pages(inode->vfs_inode.i_mapping, inode->vfs_inode.i_size);
465465
clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
466466
ret = btrfs_drop_verity_items(inode);
@@ -585,7 +585,7 @@ static int btrfs_begin_enable_verity(struct file *filp)
585585
struct btrfs_trans_handle *trans;
586586
int ret;
587587

588-
ASSERT(inode_is_locked(file_inode(filp)));
588+
btrfs_assert_inode_locked(inode);
589589

590590
if (test_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags))
591591
return -EBUSY;
@@ -633,7 +633,7 @@ static int btrfs_end_enable_verity(struct file *filp, const void *desc,
633633
int ret = 0;
634634
int rollback_ret;
635635

636-
ASSERT(inode_is_locked(file_inode(filp)));
636+
btrfs_assert_inode_locked(inode);
637637

638638
if (desc == NULL)
639639
goto rollback;

fs/btrfs/xattr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ int btrfs_setxattr(struct btrfs_trans_handle *trans, struct inode *inode,
120120
* locks the inode's i_mutex before calling setxattr or removexattr.
121121
*/
122122
if (flags & XATTR_REPLACE) {
123-
ASSERT(inode_is_locked(inode));
123+
btrfs_assert_inode_locked(BTRFS_I(inode));
124124
di = btrfs_lookup_xattr(NULL, root, path,
125125
btrfs_ino(BTRFS_I(inode)), name, name_len, 0);
126126
if (!di)

0 commit comments

Comments
 (0)