linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: [PATCHv6 02/15] writeback: fix possible race when creating bdi threads
Date: Sun, 25 Jul 2010 14:29:12 +0300	[thread overview]
Message-ID: <1280057365-10297-3-git-send-email-dedekind1@gmail.com> (raw)
In-Reply-To: <1280057365-10297-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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 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 4e9ed2a..327e36d 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-25 11:29 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-25 11:29 [PATCHv6 00/15] kill unnecessary bdi wakeups + cleanups Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 01/15] writeback: harmonize writeback threads naming Artem Bityutskiy
2010-07-25 11:29 ` Artem Bityutskiy [this message]
2010-07-25 11:29 ` [PATCHv6 03/15] writeback: do not lose wake-ups in the forker thread - 1 Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 04/15] writeback: do not lose wake-ups in the forker thread - 2 Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 05/15] writeback: do not lose wake-ups in bdi threads Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 06/15] writeback: simplify bdi code a little Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 07/15] writeback: do not remove bdi from bdi_list Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 08/15] writeback: move last_active to bdi Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 09/15] writeback: restructure bdi forker loop a little Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 10/15] writeback: move bdi threads exiting logic to the forker thread Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 11/15] writeback: prevent unnecessary bdi threads wakeups Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 12/15] writeback: optimize periodic bdi thread wakeups Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 13/15] writeback: remove unnecessary init_timer call Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 14/15] writeback: add new tracepoints Artem Bityutskiy
2010-07-25 11:29 ` [PATCHv6 15/15] writeback: cleanup bdi_register Artem Bityutskiy
2010-08-03  4:44 ` [PATCHv6 00/15] kill unnecessary bdi wakeups + cleanups Artem Bityutskiy
2010-08-03 12:27 ` Jens Axboe
2010-08-03 12:37   ` Artem Bityutskiy
2010-08-03 12:47     ` Jens Axboe
2010-08-04 11:34       ` Jens Axboe
2010-08-05  9:35         ` Artem Bityutskiy
2010-08-03 14:11   ` 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=1280057365-10297-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).