Re: [Ext2-devel] [PATCH] JBD: fix against journal overflow

[Stephen C. Tweedie]

Hi,

On Mon, 2005-01-24 at 19:27, Alex Tomas wrote:

> oops. i overlooked this line. so, the fix becomes minor improvement patch ;)

Agreed, but a worthwhile one anyway.  I'm still worried if you've seen
tests where this patch definitely cured a journal overflow, though ---
if so, it may be masking some other bug somewhere else.

> do you think the following can be improved?
> 
> 	/*
> 	 * @@@ AKPM: This seems rather over-defensive.  We're giving commit
> 	 * a _lot_ of headroom: 1/4 of the journal plus the size of
> 	 * the committing transaction.  
...

Possibly.  But what this bit is doing is effectively chunking the
checkpoint operations --- if we have run out of journal space, we wait
until we've got enough room left for a maximally-sized transaction
before we let the new transaction start.  And by doing that, we make
sure that the new transaction can grow to its full size before a commit
is forced.

The trouble is, it is not possible to wait for more log space during a
transaction's lifetime.  You can't clear old log entries without making
sure that the buffers in them have been written back elsewhere, either
to more recent transactions in the log, or to the final writeback
store.  And if you've got a buffer touched both in the oldest
transaction and the current one, then it's being journaled so you can't
do writeback; so you can't flush the old transaction from the log until
you've finished running and committing the current one.  

So if we want to allow a transaction to grow to its full size, we *must*
wait for the log to have enough space for a maximal transaction before
we let the transaction start in the first place.  And obviously, at that
point we don't know how large the transaction is going to get, so we
can't tell in advance whether we would be able to get away with a
smaller amount of space. :)

We could in theory monitor the usage, and if all the transactions being
committed are much smaller than maximum, we could shrink the space
requirement here.  But I'm not convinced it's really worth it.  (It
might be a small improvement for things like mail servers, though.)

Cheers,
 Stephen

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