[Ocfs2-devel] [PATCH -v2 1/3] jbd2: Use atomic variables to avoid taking t_handle_lock in jbd2_journal_stop

[Jan Kara]

On Tue 10-08-10 12:30:46, Ted Ts'o wrote:
> On Mon, Aug 09, 2010 at 09:45:55PM +0200, Jan Kara wrote:
> >   Ah, OK. You're right. I just thought we eventually want to remove the
> > lock but you're right that currently the code is fine. Sorry for the noise.
> 
> I would love to get rid of the j_state_lock, but looking through the
> code, I couldn't figure out how to do this safely.  Hence my
> conversion of the j_state_lock to a rwlock_t, with the downside of
> this causing more cache line bounces.  If someone can suggest a way to
> drop needing a global spinlock (whether it is an exclusive or rwlock)
> in start_this_handle(), I'd love to hear them.
  Thinking about this, I think there's a way:
1) Make transaction structures freed by RCU so when we get transaction
pointer from a journal we can operate on it without being afraid of
touching freed structure.
2) Increment t_updates count before doing anything else - with this, we are
sure that if a transaction is in LOCKED state or in some earlier state, it
won't proceed further before we drop our reference.
3) smp_mb() to get values from transaction structure after the ref count
increase.
4) Check that the transaction is actually running and  has enough credits.
5) The check
        if (__jbd2_log_space_left(journal) < jbd_space_needed(journal)) {
   seems just useless if you look at it more in detail. It just transforms
   to
     7/8*(journal->j_free-32) < journal->j_max_transaction_buffers +
                 journal->j_committing_transaction->t_outstanding_credits
   So it just doesn't seem to make sense to call it whenever we get a
handle. It should be enough to do the check only when we really start a
new transaction. If it is satisfied at that moment, it should be satisfied
during the whole lifetime of a transaction.

  This should make the fast path (when a transaction does not need to be
started) of start_this_handle() lockless. Of course when any of the checks
fails, we have to bite the bullet and take the lock.
  I can have a look into transforming this ideas into a patch but I'm not
sure when I get to it...

								Honza
-- 
Jan Kara <jack at suse.cz>
SUSE Labs, CR