mirror of
git://git.yoctoproject.org/linux-yocto.git
synced 2025-10-23 07:23:12 +02:00
btrfs: avoid load/store tearing races when checking if an inode was logged
[ Upstream commit 986bf6ed44dff7fbae7b43a0882757ee7f5ba21b ]
At inode_logged() we do a couple lockless checks for ->logged_trans, and
these are generally safe except the second one in case we get a load or
store tearing due to a concurrent call updating ->logged_trans (either at
btrfs_log_inode() or later at inode_logged()).
In the first case it's safe to compare to the current transaction ID since
once ->logged_trans is set the current transaction, we never set it to a
lower value.
In the second case, where we check if it's greater than zero, we are prone
to load/store tearing races, since we can have a concurrent task updating
to the current transaction ID with store tearing for example, instead of
updating with a single 64 bits write, to update with two 32 bits writes or
four 16 bits writes. In that case the reading side at inode_logged() could
see a positive value that does not match the current transaction and then
return a false negative.
Fix this by doing the second check while holding the inode's spinlock, add
some comments about it too. Also add the data_race() annotation to the
first check to avoid any reports from KCSAN (or similar tools) and comment
about it.
Fixes: 0f8ce49821
("btrfs: avoid inode logging during rename and link when possible")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
564ce572c5
commit
9b1e6a7152
|
@ -3344,15 +3344,32 @@ static int inode_logged(struct btrfs_trans_handle *trans,
|
||||||
struct btrfs_key key;
|
struct btrfs_key key;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
if (inode->logged_trans == trans->transid)
|
/*
|
||||||
|
* Quick lockless call, since once ->logged_trans is set to the current
|
||||||
|
* transaction, we never set it to a lower value anywhere else.
|
||||||
|
*/
|
||||||
|
if (data_race(inode->logged_trans) == trans->transid)
|
||||||
return 1;
|
return 1;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If logged_trans is not 0, then we know the inode logged was not logged
|
* If logged_trans is not 0 and not trans->transid, then we know the
|
||||||
* in this transaction, so we can return false right away.
|
* inode was not logged in this transaction, so we can return false
|
||||||
|
* right away. We take the lock to avoid a race caused by load/store
|
||||||
|
* tearing with a concurrent btrfs_log_inode() call or a concurrent task
|
||||||
|
* in this function further below - an update to trans->transid can be
|
||||||
|
* teared into two 32 bits updates for example, in which case we could
|
||||||
|
* see a positive value that is not trans->transid and assume the inode
|
||||||
|
* was not logged when it was.
|
||||||
*/
|
*/
|
||||||
if (inode->logged_trans > 0)
|
spin_lock(&inode->lock);
|
||||||
|
if (inode->logged_trans == trans->transid) {
|
||||||
|
spin_unlock(&inode->lock);
|
||||||
|
return 1;
|
||||||
|
} else if (inode->logged_trans > 0) {
|
||||||
|
spin_unlock(&inode->lock);
|
||||||
return 0;
|
return 0;
|
||||||
|
}
|
||||||
|
spin_unlock(&inode->lock);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If no log tree was created for this root in this transaction, then
|
* If no log tree was created for this root in this transaction, then
|
||||||
|
|
Loading…
Reference in New Issue
Block a user