On 2/3/20 8:10 AM, Nikolay Borisov wrote:
On 1.02.20 г. 0:36 ч., Josef Bacik wrote:
Right now if the space is free'd up after the ordered extents complete
(which is likely since the reservations are held until they complete),
we would do extra delalloc flushing before we'd notice that we didn't
have any more tickets. Fix this by moving the tickets check after our
wait_ordered_extents check.
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
This patch makes sense only for metadata. Is this your intention -
tweaking the metadata change behavior? Correct me if I'm wrong but
btrfs_start_delalloc_roots from previous patch will essentially call
btrfs_run_delalloc_range for the roots which will create ordered extents in:
btrfs_run_delalloc_range
cow_file_range
add_ordered_extents
Following page writeout, from the endio routines we'll eventually do:
finish_ordered_fn
btrfs_finish_ordered_io
insert_reserved_file_extent
btrfs_alloc_reserved_file_extent
create delayed ref <---- after the delayed extent is run this will
free some data space. But this happens in transaction commit context and
not when runnig ordered extents
btrfs_remove_ordered_extent
btrfs_delalloc_release_metadata <- this is only for metadata
btrfs_inode_rsv_release
__btrfs_block_rsv_release <-- frees metadata but not data?
I'm looking those patches thinking every change should be pertinent to
data space but apparently it's not. If so I think it will be best if you
update the cover letter for V2 to mention which patches can go in
independently or give more context why this particular patch is
pertinent to data flush.
Specifically what I'm addressing here for data is this behavior
btrfs_start_delalloc_roots()
->check space_info->tickets, it's not empty
btrfs_finish_ordered_io()
-> we drop a previously uncompressed extent (ie larger one) for a newly
compressed extent, now space_info->tickets _is_ empty
loop again despite having no tickets to flush for.
Does this scenario happen all the time? Nope, but I noticed it was happening
sometimes with the data intensive enopsc tests xfstests so I threw it in there
because it made a tiny difference, plus it's actually correct.
It definitely affects the metadata case more for sure, but I only noticed it
when I forgot I had compression enabled for one of my xfstests runs. Thanks,
Joosef
