On Wed, Oct 07, 2009 at 05:17:05PM +0800, Nick Piggin wrote:
> On Wed, Oct 07, 2009 at 05:07:22PM +0800, Wu Fengguang wrote:
> > On Wed, Oct 07, 2009 at 04:53:20PM +0800, Peter Zijlstra wrote:
> > > wakeup implies a full memory barrier.
> >
> > If so, this smp_mb__after_clear_bit() line is also not necessary?
> >
> > void clear_bdi_congested(struct backing_dev_info *bdi, int sync)
> > {
> > //...
> > clear_bit(bit, &bdi->state);
> > smp_mb__after_clear_bit();
> > if (waitqueue_active(wqh))
> > wake_up(wqh);
>
> Typically in these patterns you do need a barrier. You need to
> ensure the load to check the waitqueue is performed after the
> clear_bit.
>
> The other side does:
> add_to_waitqueue
> set_current_state /* has mb() in it */
> if (test_bit())
> schedule()
Thanks, just moved the mb() up to follow clear_bit():
if (test_bit(BDI_sync_congested, &bdi->state)) {
clear_bdi_congested(bdi, BLK_RW_SYNC);
smp_mb__after_clear_bit();
}
if (waitqueue_active(&wqh[BLK_RW_SYNC]))
wake_up(&wqh[BLK_RW_SYNC]);
Thanks,
Fengguang
--
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
