writeback: fix possible race when creating bdi threads (94eac5e6) · Commits · 戴 / test

This patch fixes a very unlikely race condition on the bdi forker thread error path: when bdi thread creation fails, 'bdi->wb.task' may contain the error code for a short period of time. If at the same time someone submits a work to this bdi, we can end up with an oops 'bdi_queue_work()' while executing 'wake_up_process(wb->task)'. This patch fixes the issue by introducing a temporary variable 'task' and storing the possible error code there, so that 'wb->task' would never take erroneous values. Note, this race is very unlikely and I never hit it, so it is theoretical, but nevertheless worth fixing. This patch also merges 2 comments which were previously separate. Signed-off-by:

Artem Bityutskiy <Artem.Bityutskiy@nokia.com> Reviewed-by:

Christoph Hellwig <hch@lst.de> Signed-off-by:

Jens Axboe <jaxboe@fusionio.com>

mm/backing-dev.c

+11 −17

Original line number	Diff line number	Diff line
		@@ -331,8 +331,8 @@ static int bdi_forker_thread(void *ptr)
		set_user_nice(current, 0);

		for (;;) {
		struct task_struct *task;
		struct backing_dev_info bdi, tmp;
		struct bdi_writeback *wb;

		/*
		* Temporary measure, we want to make sure we don't see
		@@ -383,29 +383,23 @@ static int bdi_forker_thread(void *ptr)
		list_del_init(&bdi->bdi_list);
		spin_unlock_bh(&bdi_lock);

		wb = &bdi->wb;
		wb->task = kthread_run(bdi_writeback_thread, wb, "flush-%s",
		task = kthread_run(bdi_writeback_thread, &bdi->wb, "flush-%s",
		dev_name(bdi->dev));
		if (IS_ERR(task)) {
		/*
		* If thread 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(wb->task)) {
		wb->task = NULL;

		/*
		* Add this 'bdi' to the back, so we get
		* a chance to flush other bdi's to free
		* memory.
		* If thread creation fails, then readd the bdi back to
		* the list and force writeout of the bdi from this
		* forker thread. That will free some memory and we can
		* try again. Add it to the tail so we get a chance to
		* flush other bdi's to free memory.
		*/
		spin_lock_bh(&bdi_lock);
		list_add_tail(&bdi->bdi_list, &bdi_pending_list);
		spin_unlock_bh(&bdi_lock);

		bdi_flush_io(bdi);
		}
		} else
		bdi->wb.task = task;
		}

		return 0;

Admin message