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@xxxxxxxx>
Tested-by: Nikolay Borisov <nborisov@xxxxxxxx>
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
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
