lkml.org 
[lkml]   [2003]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: ext2 FS corruption with 2.5.59.

Hi,

> +static inline void i_size_write(struct inode * inode, loff_t i_size)
> +{
> +#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> +#ifdef __ARCH_HAS_GET_SET_64BIT
> + set_64bit((unsigned long long *) &inode->i_size, (unsigned long long) i_size);
> +#else
> + inode->i_size_version2++;
> + wmb();
> + inode->i_size = i_size;
> + wmb();
> + inode->i_size_version1++;
> + wmb(); /* make it visible ASAP */
> +#endif
> +#elif BITS_PER_LONG==64 || !defined(CONFIG_SMP)
> + inode->i_size = i_size;
> +#endif
> +}

That last wmb is suspect. We dont put an wmb after a spinlock to "make it
visible". If you think of an wmb as an ordering tag that propagates out
through the cpu and storage hierarchy then wmb is not going to help us here.

I guess the store could get reordered (and so delayed) a bit, but an wmb
is relatively expensive on some architectures.

> This is actually fairly pointless, because these fields are write-mostly and
> read-rarely. But we need a spinlock anyway because of the concurrent
> modifiers problem.

It would be interesting to compare a spinlock or rwlock against a frlock
in a write mostly situation. Actually isnt it going to be slower because
we have to take a spinlock to serialise around the frlock write path?

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

\
 
 \ /
  Last update: 2005-03-22 13:32    [W:0.099 / U:3.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site