Messages in this thread | | | Date | Mon, 27 Jan 2003 15:59:39 -0800 | From | William Lee Irwin III <> | Subject | Re: ext2 FS corruption with 2.5.59. |
| |
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/
| |