On Tue 19-05-20 18:44:25, Andreas Dilger wrote:
On May 18, 2020, at 3:21 AM, Jan Kara jack@suse.cz wrote:
When reserved transaction handle is unused, we subtract its reserved credits in __jbd2_journal_unreserve_handle() called from jbd2_journal_stop(). However this function forgets to remove reserved credits from transaction->t_outstanding_credits and thus the transaction space that was reserved remains effectively leaked. The leaked transaction space can be quite significant in some cases and leads to unnecessarily small transactions and thus reducing throughput of the journalling machinery. E.g. fsmark workload creating lots of 4k files was observed to have about 20% lower throughput due to this when ext4 is mounted with dioread_nolock mount option.
Subtract reserved credits from t_outstanding_credits as well.
CC: stable@vger.kernel.org Fixes: 8f7d89f36829 ("jbd2: transaction reservation support") Signed-off-by: Jan Kara jack@suse.cz
Patch looks reasonable, with one minor nit below.
Reviewed-by: Andreas Dilger adilger@dilger.ca
Thanks!
@@ -721,8 +728,10 @@ static void stop_this_handle(handle_t *handle) } atomic_sub(handle->h_total_credits, &transaction->t_outstanding_credits);
- if (handle->h_rsv_handle)
__jbd2_journal_unreserve_handle(handle->h_rsv_handle);
- if (handle->h_rsv_handle) {
__jbd2_journal_unreserve_handle(handle->h_rsv_handle,transaction);- }
There isn't any need for braces {} around this one-line if-block.
Yeah, I'm always undecided about this. I agree that in this case the code wouldn't be any less readable without the braces. So I'll remove them and resend with your tag.
Honza