Re: ext2 FS corruption with 2.5.59.

[William Lee Irwin III <wli@holomorphy.com>]

William Lee Irwin III <wli@holomorphy.com> wrote:
>>>>> Ticket locks need atomic fetch and increment. These don't look right.

On Mon, Jan 27, 2003 at 02:59:21PM -0800, Stephen Hemminger wrote:
> Atomic fetch/increment is not necessary since it is assumed that
> only a single writer is doing the increment at a time, either with a
> lock or a semaphore.  The fr_write_lock primitive incorporates the
> spinlock and the sequence number. 

Ticket locks still need atomic fetch and increment. You don't because
not only are you not implementing a ticket lock, you've got an outright
spinlock around the fetch and increment.


William Lee Irwin III <wli@holomorphy.com> wrote:
>>> 	(1) increment ->pre_sequence
>>> 	(2) wmb()
>>> 	(3) get inode->i_size
>>> 	(4) wmb() 
>>> 	(5) increment ->post_sequence
>>> 	(6) wmb()
>>> Supposing the overall scheme is sound, one of the wmb()'s is unnecessary;

On Mon, Jan 27, 2003 at 02:59:21PM -0800, Stephen Hemminger wrote:
> Each wmb() has a purpose. (2) is to make sure the first increment
> happens before the update. (4) makes sure the update happens before the
> second increment.  
> The last wmb is unnecessary. Also on many architectures, the wmb()
> disappears since writes are never reordered.

This is apparently based on some misunderstanding wrt. thinking the
sequence of events above described a read. Obviously converting (3)
to "modify inode->i_size" makes the (4) wmb() necessary.


-- wli