All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCHv2 02/11] writeback: fix possible race when creating bdi threads
Date: Wed, 21 Jul 2010 12:31:37 +0300	[thread overview]
Message-ID: <1279704706-1267-3-git-send-email-dedekind1@gmail.com> (raw)
In-Reply-To: <1279704706-1267-1-git-send-email-dedekind1@gmail.com>

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

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>
---
 mm/backing-dev.c |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 0f7b17c..75fae17 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;
-- 
1.7.1.1


  parent reply	other threads:[~2010-07-21  9:32 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-21  9:31 [PATCHv2 00/16] kill unnecessary bdi wakeups + cleanups Artem Bityutskiy
2010-07-21  9:31 ` [PATCHv2 01/11] writeback: harmonize writeback threads naming Artem Bityutskiy
2010-07-21  9:31 ` Artem Bityutskiy [this message]
2010-07-21 11:57   ` [PATCHv2 02/11] writeback: fix possible race when creating bdi threads Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 03/11] writeback: do not lose wake-ups in the forker thread - 1 Artem Bityutskiy
2010-07-21  9:31 ` [PATCHv2 04/11] writeback: do not lose wake-ups in the forker thread - 2 Artem Bityutskiy
2010-07-21 11:57   ` Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 05/11] writeback: do not lose wake-ups in bdi threads Artem Bityutskiy
2010-07-21  9:31 ` [PATCHv2 06/11] writeback: simplify bdi code a little Artem Bityutskiy
2010-07-21 12:01   ` Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 07/11] writeback: do not remove bdi from bdi_list Artem Bityutskiy
2010-07-21 12:02   ` Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 08/11] writeback: move last_active to bdi Artem Bityutskiy
2010-07-21  9:31 ` [PATCHv2 09/11] writeback: restructure bdi forker loop a little Artem Bityutskiy
2010-07-21 12:02   ` Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 10/11] writeback: move bdi threads exiting logic to the forker thread Artem Bityutskiy
2010-07-21 12:04   ` Christoph Hellwig
2010-07-21  9:31 ` [PATCHv2 11/11] writeback: prevent unnecessary bdi threads wakeups Artem Bityutskiy
2010-07-21 11:45   ` Artem Bityutskiy
2010-07-22  0:41     ` Dave Chinner
2010-07-22  6:50       ` Artem Bityutskiy
2010-07-22  6:50         ` Artem Bityutskiy
2010-07-22  9:00       ` Christoph Hellwig
2010-07-22  9:24         ` Artem Bityutskiy
2010-07-22 13:27         ` Artem Bityutskiy
2010-07-22 13:27           ` Artem Bityutskiy
2010-07-21 12:12   ` Christoph Hellwig
2010-07-22  3:19   ` Nick Piggin
2010-07-22  6:48     ` Artem Bityutskiy
2010-07-22  7:22       ` Tero.Kristo
2010-07-22  7:22         ` Tero.Kristo
2010-07-22  8:07         ` Nick Piggin
2010-07-22  8:05       ` Nick Piggin
2010-07-22  8:02         ` Artem Bityutskiy
2010-07-22  8:02           ` Artem Bityutskiy
2010-07-22  8:59           ` Nick Piggin
2010-07-22  9:50         ` Artem Bityutskiy
2010-07-22  9:50           ` Artem Bityutskiy
2010-07-23 15:03         ` Artem Bityutskiy
2010-07-23 15:03           ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1279704706-1267-3-git-send-email-dedekind1@gmail.com \
    --to=dedekind1@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.