Hi David,
Thank you for the confirmation and the additional information.
I feel the same that this race may not lead to serious issues, but would
rather prefer a confirmation from the developers. Thank you again for
your time!
Best Regards,
Meng
On 11/1/19 11:45 AM, David Sterba wrote:
On Tue, Oct 15, 2019 at 02:33:11PM -0400, Meng Xu wrote:
I am reporting a potential data race around the `delayed_rsv->full` field.
Thanks for the report.
[thread 1] mount a btrfs image, a kernel thread of uuid_rescan will be created
btrfs_uuid_rescan_kthread
btrfs_end_transaction
__btrfs_end_transaction
btrfs_trans_release_metadata
btrfs_block_rsv_release
__btrfs_block_rsv_release
--> [READ] else if (block_rsv != global_rsv && !delayed_rsv->full)
^^^^^^^^^^^^^^^^^
[thread 2] do a mkdir syscall on the mounted image
__do_sys_mkdir
do_mkdirat
vfs_mkdir
btrfs_mkdir
btrfs_new_inode
btrfs_insert_empty_items
btrfs_cow_block
__btrfs_cow_block
alloc_tree_block_no_bg_flush
btrfs_alloc_tree_block
btrfs_add_delayed_tree_ref
btrfs_update_delayed_refs_rsv
--> [WRITE] delayed_rsv->full = 0;
^^^^^^^^^^^^^^^^^^^^^
I could confirm that this is a data race by manually adding and adjusting
delays before the read and write statements although I am not very sure
about the implication of such a data race (e.g., crashing btrfs or causing
violations of assumptions). I would appreciate if you could help check on
this potential bug and advise whether this is a harmful data race or it
is intended.
The race is there, as the access is unprotected, but it does not seem to
have serious implications. The race is for space, which happens all the
time, and if the reservations cannot be satisfied there goes ENOSPC.
In this particular case I wonder if the uuid thread is important or if
this would happen with anything that calls btrfs_end_transaction.
Depending on the value of ->full, __btrfs_block_rsv_release decides
where to return the reservation, and block_rsv_release_bytes handles a
NULL pointer for block_rsv and if it's not NULL then it double checks
the full status under a lock.
So the unlocked and racy access is only advisory and in the worst case
the block reserve is found !full and becomes full in the meantime,
but properly handled.
I've CCed Josef to review the analysis.
