| Message ID | 20200520133119.1383-3-jack@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | jbd2: Fix leaked transaction credits | expand |
On Wed, May 20, 2020 at 03:31:19PM +0200, Jan Kara 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") > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > Signed-off-by: Jan Kara <jack@suse.cz> Thanks, applied. - Ted
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 3dccc23cf010..e91aad3637a2 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -541,17 +541,24 @@ handle_t *jbd2_journal_start(journal_t *journal, int nblocks) } EXPORT_SYMBOL(jbd2_journal_start); -static void __jbd2_journal_unreserve_handle(handle_t *handle) +static void __jbd2_journal_unreserve_handle(handle_t *handle, transaction_t *t) { journal_t *journal = handle->h_journal; WARN_ON(!handle->h_reserved); sub_reserved_credits(journal, handle->h_total_credits); + if (t) + atomic_sub(handle->h_total_credits, &t->t_outstanding_credits); } void jbd2_journal_free_reserved(handle_t *handle) { - __jbd2_journal_unreserve_handle(handle); + journal_t *journal = handle->h_journal; + + /* Get j_state_lock to pin running transaction if it exists */ + read_lock(&journal->j_state_lock); + __jbd2_journal_unreserve_handle(handle, journal->j_running_transaction); + read_unlock(&journal->j_state_lock); jbd2_free_handle(handle); } EXPORT_SYMBOL(jbd2_journal_free_reserved); @@ -722,7 +729,8 @@ 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); + __jbd2_journal_unreserve_handle(handle->h_rsv_handle, + transaction); if (atomic_dec_and_test(&transaction->t_updates)) wake_up(&journal->j_wait_updates);
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") Reviewed-by: Andreas Dilger <adilger@dilger.ca> Signed-off-by: Jan Kara <jack@suse.cz> --- fs/jbd2/transaction.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)