writeback: fix possible race when creating bdi threads

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>

1 files changed, 11 insertions, 17 deletions

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 4e9ed2a8521f..327e36d1d623 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -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",
-                                        dev_name(bdi->dev));
-                /*
-                 * 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;
-
+                task = kthread_run(bdi_writeback_thread, &bdi->wb, "flush-%s",
+                                   dev_name(bdi->dev));
+                if (IS_ERR(task)) {
                         /*
-                         * 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;