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@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
