2005-01-25 00:13:57

by Alex Tomas

[permalink] [raw]
Subject: Re: [Ext2-devel] [PATCH] JBD: journal_release_buffer()

>>>>> Stephen C Tweedie (SCT) writes:

>> + /* return credit back to the handle if it was really spent */
>> + if (credits) {
>> + handle->h_buffer_credits++;
>> + spin_lock(&handle->h_transaction->t_handle_lock);
>> + handle->h_transaction->t_outstanding_credits++;
>> + spin_lock(&handle->h_transaction->t_handle_lock);
>> + }

SCT> That returns the credit to A (satisfying ext3), but you just grew
SCT> t_outstanding_credits, thus growing the journal commitments without
SCT> checking if it's safe to do so or being able to handle failure. So it
SCT> just reintroduces the original bug.

incremented h_buffer_credits will be subtracted from incremented
t_outstanding_credits in journal_stop(). so, there is no imbalance
at this point.

then, if (b_tcount == 0) we can correct t_outstanind_credits or
h_buffer_credits. wrong?

let's try another way ... we have two processes: P1 and P2. they
access block B.

the code is:
if (credits != 0) {
handle->h_buffer_credits++;
transaction->t_outstanding_credits++;
}
if (jh->b_tcount == 0)
transcation->t_outstanding_credits--;

case 1:
P1 accesses B (*credits=1)
P1 releases B

(credits != 0) h1->h_buffer_credits++;
(credits != 0) transaction->t_outstanding_credits++;
(b_tcount == 0) transaction->t_outstanding_credits--;

OUTPUT: transaction->t_outstanding_credits -= 1


case 2:
P1 accesses B (*credits=1)
P2 accesses B (*credits=0)
P1 releases B
P2 modifies B

(credits != 0) h1->h_buffer_credits++;
(credits != 0) transaction->t_outstainding_credits++;
(b_tcount != 0)

OUTPUT: journal_stop() will subtract incremented h_buffer_credits
from incremented t_outstading_credits => no changes


case 3:
P1 accesses B (*credits=1)
P2 accesses B (*credits=0)
P2 releases B
P1 modifies B

(credits != 0)
(credits != 0)
(b_tcount == 0)

OUTPUT: no changes


case 4:
P1 accesses B (*credits=1)
P2 accesses B (*credits=0)
P2 releases B
P1 releases B

(credits != 0)
(credits != 0)
(b_tcount == 0)

(credits != 0) h1->h_buffer_credits++;
(credits != 0) transaction->t_outstanding_credits++;
(b_tcount == 0) transaction->t_outstanding_credits--;

OUTPUT: P2 will change nothing, P1 will drop the buffer and
correct t_outstanding_credits


case 5:
P1 accesses B (*credits=1)
P2 accesses B (*credits=0)
P1 releases B
P2 releases B

(credits != 0) h1->h_buffer_credits++;
(credits != 0) transaction->t_outstanding_credits++;
(b_tcount == 0)

(credits != 0)
(credits != 0)
(b_tcount == 0) transaction->t_outstanding_credits--;

OUTPUT: P1 will change own credits, P2 will drop the buffer
and correct t_outstanding_credits



thanks, Alex