• Ritesh Harjani's avatar
    jbd2: kill t_handle_lock transaction spinlock · f7f497cb
    Ritesh Harjani authored
    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: default avatarJan Kara <jack@suse.cz>
    Signed-off-by: default avatarRitesh Harjani <riteshh@linux.ibm.com>
    Reviewed-by: default avatarJan Kara <jack@suse.cz>
    Link: https://lore.kernel.org/r/d89e599658b4a1f3893a48c6feded200073037fc.1644992076.git.riteshh@linux.ibm.comSigned-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
    f7f497cb
transaction.c 85.4 KB