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

[Alex Tomas <alex@clusterfs.com>]

>>>>> 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