Re: ext2 FS corruption with 2.5.59.

From: William Lee Irwin III (wli@holomorphy.com)
Date: Mon Jan 27 2003 - 18:59:39 EST


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



This archive was generated by hypermail 2b29 : Fri Jan 31 2003 - 22:00:17 EST