Messages in this thread | | | Date | Tue, 07 Jul 2009 18:27:09 +0300 | From | Artem Bityutskiy <> | Subject | Re: [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/
| |