On Wed, 2009-05-27 at 11:41 +0200, Jens Axboe wrote:
> + if (writeback_acquire(bdi)) {
> + bdi->wb_arg.nr_pages = nr_pages;
> + bdi->wb_arg.sb = sb;
> + bdi->wb_arg.sync_mode = sync_mode;
> + /*
> + * make above store seen before the task is woken
> + */
> + smp_mb();
> + wake_up(&bdi->wait);
> + }
wake_up() implies a wmb() when we indeed to a wakeup, is that
sufficient?
> +int bdi_writeback_task(struct backing_dev_info *bdi)
> +{
> + while (!kthread_should_stop()) {
> + unsigned long wait_jiffies;
> + DEFINE_WAIT(wait);
> +
> + prepare_to_wait(&bdi->wait, &wait, TASK_INTERRUPTIBLE);
> + wait_jiffies = msecs_to_jiffies(dirty_writeback_interval * 10);
> + schedule_timeout(wait_jiffies);
> + try_to_freeze();
> +
> + /*
> + * We get here in two cases:
> + *
> + * schedule_timeout() returned because the dirty writeback
> + * interval has elapsed. If that happens, we will be able
> + * to acquire the writeback lock and will proceed to do
> + * kupdated style writeout.
> + *
> + * Someone called bdi_start_writeback(), which will acquire
> + * the writeback lock. This means our writeback_acquire()
> + * below will fail and we call into bdi_pdflush() for
> + * pdflush style writeout.
> + *
> + */
> + if (writeback_acquire(bdi))
> + bdi_kupdated(bdi);
> + else
> + bdi_pdflush(bdi);
> +
> + writeback_release(bdi);
> + finish_wait(&bdi->wait, &wait);
> + }
> +
> + return 0;
> +}
the unpaired writeback_release() wrt writeback_acquire() looks odd.
Also the prepare/finish wait bits seem oddly out of place. Are there
really multiple waiters on bdi->wait? The above wake_up() seems to
suggest not, since it directly modifies bdi state instead of queueing
work.
--
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
