Nick, thanks for serialization suggestion.
On Thu, 2010-05-27 at 17:22 +1000, Nick Piggin wrote:
> Yeah, we definitely don't want to add global cacheline writes in the
> common case. Also I don't know why you do the strange -1 value. I
> couldn't seem to find where you defined bdi_arm_supers_timer();
It is in mm/backing-dev.c:376 in today's Linus' tree. The -1 is used to
indicate that 'sync_supers()' is in progress and avoid arming timer in
that case. But yes, this is not really needed.
> But why doesn't this work?
>
> sb->s_dirty = 1;
> smp_mb(); /* corresponding MB is in test_and_clear_bit */
AFAIU, test_and_clear_bit assumes 2 barriers - before the test and after
the clear. Then I do not really understand why this smp_mb is needed.
> if (unlikely(!supers_timer_armed)) {
> if (!test_and_set_bit(0, &supers_timer_armed))
> bdi_arm_supers_timer();
> }
>
> vs
>
> supers_timer_armed = 0;
> again:
> sync_supers();
> if (test_and_clear_bit(0, &supers_timer_armed))
> goto again;
AFAIU, the following should be fine, no?:
if (unlikely(!supers_timer_armed))
if (!test_and_set_bit(0, &supers_timer_armed))
bdi_arm_supers_timer();
vs
again:
sync_supers();
if (test_and_clear_bit(0, &supers_timer_armed))
goto again;
I assume that it is harmless to run 'bdi_arm_supers_timer()'
concurrently;
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
