Re: [PATCH] btrfs: do not block starts waiting on previous transaction commit

[David Sterba <dsterba@suse.cz>]

On Thu, Aug 24, 2023 at 04:59:22PM -0400, Josef Bacik wrote:
> Internally I got a report of very long stalls on normal operations like
> creating a new file when auto relocation was running.  The reporter used
> the bpf offcputime tracer to show that we would get stuck in
> start_transaction for 5 to 30 seconds, and were always being woken up by
> the transaction commit.
> 
> Using my timing-everything script, which times how long a function takes
> and what percentage of that total time is taken up by its children, I
> saw several traces like this
> 
> 1083 took 32812902424 ns
>         29929002926 ns 91.2110% wait_for_commit_duration
>         25568 ns 7.7920e-05% commit_fs_roots_duration
>         1007751 ns 0.00307% commit_cowonly_roots_duration
>         446855602 ns 1.36182% btrfs_run_delayed_refs_duration
>         271980 ns 0.00082% btrfs_run_delayed_items_duration
>         2008 ns 6.1195e-06% btrfs_apply_pending_changes_duration
>         9656 ns 2.9427e-05% switch_commit_roots_duration
>         1598 ns 4.8700e-06% btrfs_commit_device_sizes_duration
>         4314 ns 1.3147e-05% btrfs_free_log_root_tree_duration
> 
> Here I was only tracing functions that happen where we are between
> START_COMMIT and UNBLOCKED in order to see what would be keeping us
> blocked for so long.  The wait_for_commit() we do is where we wait for a
> previous transaction that hasn't completed it's commit.  This can
> include all of the unpin work and other cleanups, which tends to be the
> longest part of our transaction commit.
> 
> There is no reason we should be blocking new things from entering the
> transaction at this point, it just adds to random latency spikes for no
> reason.
> 
> Fix this by adding a PREP stage.  This allows us to properly deal with
> multiple committers coming in at the same time, we retain the behavior
> that the winner waits on the previous transaction and the losers all
> wait for this transaction commit to occur.  Nothing else is blocked
> during the PREP stage, and then once the wait is complete we switch to
> COMMIT_START and all of the same behavior as before is maintained.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Interesting, so splitting the state to 2 makes N threads race for the
first half and the winner proceed, not to race with any of the other
while progressing in the transaction flow. This resembles the MCS
spinlocks, the state is split into the owner of the state and the other
threads that have queued to acquire it. The transaction state is a bit
different, heavier than a spin lock, but still there's an atomic switch
once the winner finishes the work and lets the other threads race again.

Added to misc-next, thanks. As this is changing the core mechanism of a
transaction I'd rather let it get tested for a longer time so
technically this is a fix will wait a bit.