linux-kernel - Re: [PATCH AUTOSEL 6.5 13/41] btrfs: do not block starts waiting on previous transaction commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230925130112.GK13697@suse.cz>
Date:   Mon, 25 Sep 2023 15:01:12 +0200
From:   David Sterba <dsterba@...e.cz>
To:     Sasha Levin <sashal@...nel.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Josef Bacik <josef@...icpanda.com>,
        Filipe Manana <fdmanana@...e.com>,
        David Sterba <dsterba@...e.com>, clm@...com,
        linux-btrfs@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 6.5 13/41] btrfs: do not block starts waiting on
 previous transaction commit

On Sun, Sep 24, 2023 at 09:15:01AM -0400, Sasha Levin wrote:
> From: Josef Bacik <josef@...icpanda.com>
> 
> [ Upstream commit 77d20c685b6baeb942606a93ed861c191381b73e ]
> 
> 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.
> 
> Reviewed-by: Filipe Manana <fdmanana@...e.com>
> Signed-off-by: Josef Bacik <josef@...icpanda.com>
> Reviewed-by: David Sterba <dsterba@...e.com>
> Signed-off-by: David Sterba <dsterba@...e.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>

Please postpone adding this patch to stable trees until 6.6 is
released. Thanks.