From: Josef Bacik <josef@toxicpanda.com>
To: Nikolay Borisov <nborisov@suse.com>,
linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction
Date: Tue, 7 Jul 2020 11:01:40 -0400 [thread overview]
Message-ID: <040d407f-53ab-9f62-05ac-3b85aa71ee0a@toxicpanda.com> (raw)
In-Reply-To: <216b0050-3684-9c74-16e6-8a776a23f41e@suse.com>
On 7/7/20 10:39 AM, Nikolay Borisov wrote:
>
>
> On 30.06.20 г. 16:59 ч., Josef Bacik wrote:
>> Data space flushing currently unconditionally commits the transaction
>> twice in a row, and the last time it checks if there's enough pinned
>> extents to satisfy it's reservation before deciding to commit the
>> transaction for the 3rd and final time.
>>
>> Encode this logic into may_commit_transaction(). In the next patch we
>> will pass in U64_MAX for bytes_needed the first two times, and the final
>> time we will pass in the actual bytes we need so the normal logic will
>> apply.
>>
>> This patch exists soley to make the logical changes I will make to the
>> flushing state machine separate to make it easier to bisect any
>> performance related regressions.
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>> Tested-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>
> On a second pass through I'm now somewhat reluctant to merge this code.
> may_commit_transaction has grown more logic which pertain solely to
> metadata. As such I think we should separate that logic (i.e current
> may_commit_transaction) and any further adjustments we might want to
> make for data space info. For example all the delayed refs/trans
> reservation checks etc. make absolutely no sense for data space info.
Oooh I forgot another thing, the reason I did this is because I was trying to
maintain the behavior while converting, and then change the behavior in
subsequent patches. I'm doing this specifically because we may have performance
regressions, and I didn't want a bisect to show up at "Giant patch that changes
all the things". I want it to show up at the patch that actually changed the
behavior, so we have a better idea of where to look. So this intermediate step
is a bit wonky for sure, but the end result at the end of the series is less
wonky. But I'm still going to update that comment. Thanks,
Josef
next prev parent reply other threads:[~2020-07-07 15:01 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-30 13:58 [PATCH 00/23] Change data reservations to use the ticketing infra Josef Bacik
2020-06-30 13:58 ` [PATCH 01/23] btrfs: change nr to u64 in btrfs_start_delalloc_roots Josef Bacik
2020-06-30 13:59 ` [PATCH 02/23] btrfs: remove orig from shrink_delalloc Josef Bacik
2020-06-30 13:59 ` [PATCH 03/23] btrfs: handle U64_MAX for shrink_delalloc Josef Bacik
2020-06-30 13:59 ` [PATCH 04/23] btrfs: make shrink_delalloc take space_info as an arg Josef Bacik
2020-06-30 13:59 ` [PATCH 05/23] btrfs: make ALLOC_CHUNK use the space info flags Josef Bacik
2020-06-30 13:59 ` [PATCH 06/23] btrfs: call btrfs_try_granting_tickets when freeing reserved bytes Josef Bacik
2020-06-30 13:59 ` [PATCH 07/23] btrfs: call btrfs_try_granting_tickets when unpinning anything Josef Bacik
2020-06-30 13:59 ` [PATCH 08/23] btrfs: call btrfs_try_granting_tickets when reserving space Josef Bacik
2020-06-30 13:59 ` [PATCH 09/23] btrfs: use the btrfs_space_info_free_bytes_may_use helper for delalloc Josef Bacik
2020-06-30 13:59 ` [PATCH 10/23] btrfs: use btrfs_start_delalloc_roots in shrink_delalloc Josef Bacik
2020-06-30 13:59 ` [PATCH 11/23] btrfs: check tickets after waiting on ordered extents Josef Bacik
2020-06-30 13:59 ` [PATCH 12/23] btrfs: add flushing states for handling data reservations Josef Bacik
2020-06-30 13:59 ` [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction Josef Bacik
2020-07-07 14:39 ` Nikolay Borisov
2020-07-07 14:54 ` Josef Bacik
2020-07-07 15:01 ` Josef Bacik [this message]
2020-06-30 13:59 ` [PATCH 14/23] btrfs: add btrfs_reserve_data_bytes and use it Josef Bacik
2020-06-30 13:59 ` [PATCH 15/23] btrfs: use ticketing for data space reservations Josef Bacik
2020-07-07 14:46 ` Nikolay Borisov
2020-07-07 14:56 ` Josef Bacik
2020-06-30 13:59 ` [PATCH 16/23] btrfs: serialize data reservations if we are flushing Josef Bacik
2020-06-30 13:59 ` [PATCH 17/23] btrfs: use the same helper for data and metadata reservations Josef Bacik
2020-06-30 13:59 ` [PATCH 18/23] btrfs: drop the commit_cycles stuff for data reservations Josef Bacik
2020-06-30 13:59 ` [PATCH 19/23] btrfs: don't force commit if we are data Josef Bacik
2020-06-30 13:59 ` [PATCH 20/23] btrfs: run delayed iputs before committing the transaction for data Josef Bacik
2020-06-30 13:59 ` [PATCH 21/23] btrfs: flush delayed refs when trying to reserve data space Josef Bacik
2020-06-30 13:59 ` [PATCH 22/23] btrfs: do async reclaim for data reservations Josef Bacik
2020-06-30 13:59 ` [PATCH 23/23] btrfs: add a comment explaining the data flush steps Josef Bacik
2020-07-03 16:30 ` [PATCH 00/23] Change data reservations to use the ticketing infra David Sterba
2020-07-07 15:28 ` Josef Bacik
-- strict thread matches above, loose matches on Subject: below --
2020-07-21 14:22 [PATCH 00/23][v4] " Josef Bacik
2020-07-21 14:22 ` [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction Josef Bacik
2020-08-13 16:35 ` David Sterba
2020-07-08 13:59 [PATCH 00/23][v3] Change data reservations to use the ticketing infra Josef Bacik
2020-07-08 14:00 ` [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction Josef Bacik
2020-07-07 15:42 [PATCH 00/23][v2] Change data reservations to use the ticketing infra Josef Bacik
2020-07-07 15:42 ` [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction Josef Bacik
2020-02-04 16:19 [PATCH 0/23][v4] Convert data reservations to the ticketing infrastructure Josef Bacik
2020-02-04 16:19 ` [PATCH 13/23] btrfs: add the data transaction commit logic into may_commit_transaction Josef Bacik
2020-02-04 16:56 ` Johannes Thumshirn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=040d407f-53ab-9f62-05ac-3b85aa71ee0a@toxicpanda.com \
--to=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox