jbd2: kill t_handle_lock transaction spinlock
authorRitesh Harjani <riteshh@linux.ibm.com>
Wed, 16 Feb 2022 07:00:35 +0000 (12:30 +0530)
committerTheodore Ts'o <tytso@mit.edu>
Sat, 26 Feb 2022 02:28:13 +0000 (21:28 -0500)
commitf7f497cb702462e8505ff3d8d4e7722ad95626a1
tree5bedaec0e0276f9ff333f61affb25ba7c0aa3433
parentcc16eecae687912238ee6efbff71ad31e2bc414e
jbd2: kill t_handle_lock transaction spinlock

This patch kills t_handle_lock transaction spinlock completely from
jbd2.

To explain the reasoning, currently there were three sites at which
this spinlock was used.

1. jbd2_journal_wait_updates()
   a. Based on careful code review it can be seen that, we don't need this
      lock here. This is since we wait for any currently ongoing updates
      based on a atomic variable t_updates. And we anyway don't take any
      t_handle_lock while in stop_this_handle().
      i.e.

write_lock(&journal->j_state_lock()
jbd2_journal_wait_updates()  stop_this_handle()
while (atomic_read(txn->t_updates) {  |
DEFINE_WAIT(wait);  |
prepare_to_wait();  |
if (atomic_read(txn->t_updates)  if (atomic_dec_and_test(txn->t_updates))
write_unlock(&journal->j_state_lock);
schedule(); wake_up()
write_lock(&journal->j_state_lock);
finish_wait();
   }
txn->t_state = T_COMMIT
write_unlock(&journal->j_state_lock);

   b.  Also note that between atomic_inc(&txn->t_updates) in
       start_this_handle() and jbd2_journal_wait_updates(), the
       synchronization happens via read_lock(journal->j_state_lock) in
       start_this_handle();

2. jbd2_journal_extend()
   a. jbd2_journal_extend() is called with the handle of each process from
      task_struct. So no lock required in updating member fields of handle_t

   b. For member fields of h_transaction, all updates happens only via
      atomic APIs (which is also within read_lock()).
      So, no need of this transaction spinlock.

3. update_t_max_wait()
   Based on Jan suggestion, this can be carefully removed using atomic
   cmpxchg API.
   Note that there can be several processes which are waiting for a new
   transaction to be allocated and started. For doing this only one
   process will succeed in taking write_lock() and allocating a new txn.
   After that all of the process will be updating the t_max_wait (max
   transaction wait time). This can be done via below method w/o taking
   any locks using atomic cmpxchg.
   For more details refer [1]

   new = get_new_val();
   old = READ_ONCE(ptr->max_val);
   while (old < new)
old = cmpxchg(&ptr->max_val, old, new);

[1]: https://lwn.net/Articles/849237/

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/d89e599658b4a1f3893a48c6feded200073037fc.1644992076.git.riteshh@linux.ibm.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
fs/jbd2/transaction.c
include/linux/jbd2.h