From: Artem Bityutskiy on
On Sun, 2010-06-06 at 17:50 +0300, Artem Bityutskiy wrote:
> void sb_mark_dirty(struct super_block *sb);
> static inline void sb_mark_clean(struct super_block *sb)
> {
> sb->s_dirty = 0;
> + /*
> + * Normally FSes first unset the sb->s_dirty flag, and then start
> + * synchronizing the SB. The memory barrier ensures this order.
> + */
> + smp_mb();
....
> void sb_mark_dirty(struct super_block *sb)
> {
> + /*
> + * Normally FSes modify the SB, and then mark it as dirty. The memory
> + * barrier ensures this order.
> + */
> + smp_mb();
....

Hmm, these ones should be 'mb()', not 'smp_mb()'.

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Artem Bityutskiy on
On Sun, 2010-06-06 at 20:16 +0300, Artem Bityutskiy wrote:
> On Sun, 2010-06-06 at 17:50 +0300, Artem Bityutskiy wrote:
> > void sb_mark_dirty(struct super_block *sb);
> > static inline void sb_mark_clean(struct super_block *sb)
> > {
> > sb->s_dirty = 0;
> > + /*
> > + * Normally FSes first unset the sb->s_dirty flag, and then start
> > + * synchronizing the SB. The memory barrier ensures this order.
> > + */
> > + smp_mb();
> ...
> > void sb_mark_dirty(struct super_block *sb)
> > {
> > + /*
> > + * Normally FSes modify the SB, and then mark it as dirty. The memory
> > + * barrier ensures this order.
> > + */
> > + smp_mb();
> ...
>
> Hmm, these ones should be 'mb()', not 'smp_mb()'.

Actually no, sorry, I completely missed that all memory barriers are a
compiler barriers. I thought smp_mb() is nought, which is not true -
smp_mb() is a barrier() on UP.

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Artem Bityutskiy on
On Sun, 2010-06-06 at 17:50 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <Artem.Bityutskiy(a)nokia.com>
>
> The proper way for file-systems to synchronize the superblock
> should be as follows:
>
> 1. when modifying the SB, first modify it, then mark it as dirty;
> 2. when synchronizing the SB, first mark as clean, then start
> synchronizing.
>
> And to make ensure the order, we need memory barriers in 'sb_mark_clean()'
> and 'sb_mark_dirty()'.

I believe this stuff is a separate story, and should be handled
separately. I'll keep this separately from the 'sync_supers()' wakes up
optimization.

I actually now cannot prove myself whether these smp_mb()'s I added in
this patch make sense or not, and whether the races in FSes I was trying
to address can be addressed without spinlocks. Really dunno - but I will
keep trying to get better understanding. Reading
Documentation/memory-barriers.txt and some McKenny's docs only did not
help so far :-)

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/