lkml.org 
[lkml]   [2009]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 02/10] writeback: switch to per-bdi threads for flushing data
Hi Jens, I've noticed the following _possible_ issue.

Jens Axboe wrote:
> +static int bdi_forker_task(void *ptr)
> +{
> + struct backing_dev_info *me = ptr;
> +
> + for (;;) {
> + struct backing_dev_info *bdi, *tmp;
> +
> + /*
> + * Temporary measure, we want to make sure we don't see
> + * dirty data on the default backing_dev_info
> + */
> + if (bdi_has_dirty_io(me))
> + bdi_flush_io(me);
> +
> + spin_lock(&bdi_lock);
> +
> + /*
> + * Check if any existing bdi's have dirty data without
> + * a thread registered. If so, set that up.
> + */
> + list_for_each_entry_safe(bdi, tmp, &bdi_list, bdi_list) {
> + if (bdi->task || !bdi_has_dirty_io(bdi))
> + continue;
> +
> + bdi_add_default_flusher_task(bdi);
> + }
> +
> + set_current_state(TASK_INTERRUPTIBLE);
> +
What happens if we are preempted here? Since we have TASK_INTERRUPTIBLE
state, we will not come back unless some other task wakes us up. Who
would wake us up in this case?

> + if (list_empty(&bdi_pending_list)) {
> + unsigned long wait;
> +
> + spin_unlock(&bdi_lock);
> + wait = msecs_to_jiffies(dirty_writeback_interval * 10);
> + schedule_timeout(wait);
> + try_to_freeze();
> + continue;
> + }
> +
> + __set_current_state(TASK_RUNNING);
> +
> + /*
> + * This is our real job - check for pending entries in
> + * bdi_pending_list, and create the tasks that got added
> + */
> + bdi = list_entry(bdi_pending_list.next, struct backing_dev_info,
> + bdi_list);
> + list_del_init(&bdi->bdi_list);
> + spin_unlock(&bdi_lock);
> +
> + BUG_ON(bdi->task);
> +
> + bdi->task = kthread_run(bdi_start_fn, bdi, "flush-%s",
> + dev_name(bdi->dev));
> + /*
> + * If task creation fails, then readd the bdi to
> + * the pending list and force writeout of the bdi
> + * from this forker thread. That will free some memory
> + * and we can try again.
> + */
> + if (IS_ERR(bdi->task)) {
> + bdi->task = NULL;
> +
> + /*
> + * Add this 'bdi' to the back, so we get
> + * a chance to flush other bdi's to free
> + * memory.
> + */
> + spin_lock(&bdi_lock);
> + list_add_tail(&bdi->bdi_list, &bdi_pending_list);
> + spin_unlock(&bdi_lock);
> +
> + bdi_flush_io(bdi);
> + }
> + }
> +
> + return 0;
> +}



--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
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: 2009-07-07 17:31    [W:3.789 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site