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

[Alex Tomas]

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

 >> +	/* return credit back to the handle if it was really spent */
 >> +	if (credits)
 >> +		handle->h_buffer_credits++; 

 >> +	jh->b_tcount--;
 >> +	if (jh->b_tcount == 0) {
 >> +		/* 
 >> +		 * this was last reference to the block from the current
 >> +		 * transaction and we'd like to return credit to the
 >> +		 * whole transaction -bzzz
 >> +		 */
 >> +		if (!credits)
 >> +			handle->h_buffer_credits++; 

 SCT> I think there's a problem here.

 SCT> What if:
 SCT>   Process A gets write access, and is the first to do so (*credits=1)
 SCT>   Processes B gets write access (*credits=0)
 SCT>   B modifies the buffer and finishes
 SCT>   A looks again, sees B's modifications, and releases the buffer because
 SCT> it's no use any more.

 SCT> Now, B's release didn't return credits.  The bh is part of the
 SCT> transaction and was not released.

hmmm. that's a good catch. so, with this patch A increments h_buffer_credits
and this one will go to the t_outstanding_credits while the buffer is still
part of the transaction. indeed, an imbalance.

probably something like the following would be enough?

 +	/* 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);
 +      }

thanks, Alex


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/