2022-01-24 11:20:14

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv1 2/2] jbd2: Remove CONFIG_JBD2_DEBUG to update t_max_wait

CONFIG_JBD2_DEBUG and jbd2_journal_enable_debug knobs were added in
update_t_max_wait(), since earlier it used to take a spinlock for updating
t_max_wait, which could cause a bottleneck while starting a txn
(start_this_handle()).
Since in previous patch, we have killed t_handle_lock completely, we
could get rid of this debug config and knob to let t_max_wait be updated
by default again.

Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/jbd2/transaction.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 68dd7de49aff..77634e2e118e 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -141,20 +141,19 @@ static void jbd2_get_transaction(journal_t *journal,
* t_max_wait is carefully updated here with use of atomic compare exchange.
* Note that there could be multiplre threads trying to do this simultaneously
* hence using cmpxchg to avoid any use of locks in this case.
+ * With this t_max_wait can be updated w/o enabling jbd2_journal_enable_debug.
*/
static inline void update_t_max_wait(transaction_t *transaction,
unsigned long ts)
{
-#ifdef CONFIG_JBD2_DEBUG
unsigned long oldts, newts;
- if (jbd2_journal_enable_debug &&
- time_after(transaction->t_start, ts)) {
+
+ if (time_after(transaction->t_start, ts)) {
newts = jbd2_time_diff(ts, transaction->t_start);
oldts = READ_ONCE(transaction->t_max_wait);
while (oldts < newts)
oldts = cmpxchg(&transaction->t_max_wait, oldts, newts);
}
-#endif
}

/*
--
2.31.1


2022-01-24 18:56:43

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCHv1 2/2] jbd2: Remove CONFIG_JBD2_DEBUG to update t_max_wait

On Sun 23-01-22 22:53:28, Ritesh Harjani wrote:
> CONFIG_JBD2_DEBUG and jbd2_journal_enable_debug knobs were added in
> update_t_max_wait(), since earlier it used to take a spinlock for updating
> t_max_wait, which could cause a bottleneck while starting a txn
> (start_this_handle()).
> Since in previous patch, we have killed t_handle_lock completely, we
> could get rid of this debug config and knob to let t_max_wait be updated
> by default again.
>
> Signed-off-by: Ritesh Harjani <[email protected]>

Sounds fine. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/jbd2/transaction.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 68dd7de49aff..77634e2e118e 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -141,20 +141,19 @@ static void jbd2_get_transaction(journal_t *journal,
> * t_max_wait is carefully updated here with use of atomic compare exchange.
> * Note that there could be multiplre threads trying to do this simultaneously
> * hence using cmpxchg to avoid any use of locks in this case.
> + * With this t_max_wait can be updated w/o enabling jbd2_journal_enable_debug.
> */
> static inline void update_t_max_wait(transaction_t *transaction,
> unsigned long ts)
> {
> -#ifdef CONFIG_JBD2_DEBUG
> unsigned long oldts, newts;
> - if (jbd2_journal_enable_debug &&
> - time_after(transaction->t_start, ts)) {
> +
> + if (time_after(transaction->t_start, ts)) {
> newts = jbd2_time_diff(ts, transaction->t_start);
> oldts = READ_ONCE(transaction->t_max_wait);
> while (oldts < newts)
> oldts = cmpxchg(&transaction->t_max_wait, oldts, newts);
> }
> -#endif
> }
>
> /*
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR