From: Artem Bityutskiy <dedekind1@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFC][PATCH 12/16] writeback: add to bdi_list in the forker thread
Date: Fri, 16 Jul 2010 15:45:08 +0300 [thread overview]
Message-ID: <1279284312-2411-13-git-send-email-dedekind1@gmail.com> (raw)
In-Reply-To: <1279284312-2411-1-git-send-email-dedekind1@gmail.com>
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Before creating a bdi thread, the forker thread first removes the
corresponding bdi from the 'bdi_list', then creates the bdi thread
and wakes it up. The thread then adds itself back to the 'bdi_list'.
There is no problem with this, except that it makes the logic a
tiny bit more twisted, because the code reader has to spend some
time to figure out when the bdi is moved back. But this is minor.
However, this patch makes the forker thread add the bdi back to
the 'bdi_list' itself, rather than letting the bdi thread do this.
The reason of the change is to prepare for further changes. Namely,
the further patches will move the bdi thread exiting logic from
bdi threads to the forker thread. So the forker thread will kill
inactive bdi threads. And for the killing case, the forker thread
will have to add bdi's back to to the 'bdi_list' itself. And to make
the code consistent, it is better if we use the same approach for
the bdi thread creation path as well. This is just more elegant.
Note, bdi threads added themselves to the head of the 'bdi_list',
but in the error path the forker thread added them to the tail of
the 'bdi_list'. This patch modifies the code so that bdi's are
always added to the tail. There is no fundamental reason for this,
this is again, just for consistency and to make the code simpler.
I just do not see any problem adding them back to the tail instead
of the head. And this should even be a bit better, because the
next iteration of the bdi forker thread loop will start looking
to the 'bdi_list' from the head, and it will find a bdi which
requires attention a bit faster.
And for absolutely the same reasons, this patch also moves the
piece of code which clears the BDI_pending bit and wakes up the
waiters from bdi threads to the forker thread.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
fs/fs-writeback.c | 14 --------------
mm/backing-dev.c | 21 +++++++++++++++------
2 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 8469b93..968dc8e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -803,13 +803,6 @@ int bdi_writeback_thread(void *data)
unsigned long wait_jiffies = -1UL;
long pages_written;
- /*
- * Add us to the active bdi_list
- */
- spin_lock_bh(&bdi_lock);
- list_add_rcu(&bdi->bdi_list, &bdi_list);
- spin_unlock_bh(&bdi_lock);
-
current->flags |= PF_FLUSHER | PF_SWAPWRITE;
set_freezable();
wb->last_active = jiffies;
@@ -819,13 +812,6 @@ int bdi_writeback_thread(void *data)
*/
set_user_nice(current, 0);
- /*
- * Clear pending bit and wakeup anybody waiting to tear us down
- */
- clear_bit(BDI_pending, &bdi->state);
- smp_mb__after_clear_bit();
- wake_up_bit(&bdi->state, BDI_pending);
-
trace_writeback_thread_start(bdi);
while (!kthread_should_stop()) {
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index a42e5ef..0123d6f 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -390,21 +390,30 @@ static int bdi_forker_thread(void *ptr)
task = kthread_run(bdi_writeback_thread, &bdi->wb, "flush-%s",
dev_name(bdi->dev));
+
+ spin_lock_bh(&bdi_lock);
+ list_add_tail(&bdi->bdi_list, &bdi_list);
+ spin_unlock_bh(&bdi_lock);
+
if (IS_ERR(task)) {
/*
* 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.
+ * try again. The bdi was added to the tail so we'll
+ * get a chance to flush other bdi's to free memory.
*/
- spin_lock_bh(&bdi_lock);
- list_add_tail(&bdi->bdi_list, &bdi_list);
- spin_unlock_bh(&bdi_lock);
-
bdi_flush_io(bdi);
} else
bdi->wb.task = task;
+
+ /*
+ * Clear pending bit and wakeup anybody waiting to tear us
+ * down.
+ */
+ clear_bit(BDI_pending, &bdi->state);
+ smp_mb__after_clear_bit();
+ wake_up_bit(&bdi->state, BDI_pending);
}
return 0;
--
1.7.1.1
next prev parent reply other threads:[~2010-07-16 12:45 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-16 12:44 [RFC][PATCH 00/16] kill unnecessary bdi wakeups + cleanups Artem Bityutskiy
2010-07-16 12:44 ` [RFC][PATCH 01/16] writeback: do not self-wakeup Artem Bityutskiy
2010-07-18 6:44 ` Christoph Hellwig
2010-07-18 9:43 ` Artem Bityutskiy
2010-07-16 12:44 ` [RFC][PATCH 02/16] writeback: remove redundant list initialization Artem Bityutskiy
2010-07-18 6:44 ` Christoph Hellwig
2010-07-16 12:44 ` [RFC][PATCH 03/16] writeback: harmonize writeback threads naming Artem Bityutskiy
2010-07-18 6:45 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 04/16] writeback: fix possible race when shutting down bdi Artem Bityutskiy
2010-07-18 6:47 ` Christoph Hellwig
2010-07-20 8:58 ` Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 05/16] writeback: fix possible race when creating bdi threads Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 06/16] writeback: improve bdi_has_dirty_io Artem Bityutskiy
2010-07-18 6:49 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 07/16] writeback: do not lose wake-ups in the forker thread Artem Bityutskiy
2010-07-18 6:49 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 08/16] writeback: do not lose default bdi wake-ups Artem Bityutskiy
2010-07-18 6:52 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 09/16] writeback: do not lose wake-ups in bdi threads Artem Bityutskiy
2010-07-18 6:52 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 10/16] writeback: simplify bdi code a little Artem Bityutskiy
2010-07-18 6:56 ` Christoph Hellwig
2010-07-20 10:34 ` Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 11/16] writeback: move last_active to bdi Artem Bityutskiy
2010-07-18 7:03 ` Christoph Hellwig
2010-07-16 12:45 ` Artem Bityutskiy [this message]
2010-07-18 6:58 ` [RFC][PATCH 12/16] writeback: add to bdi_list in the forker thread Christoph Hellwig
2010-07-20 11:07 ` Artem Bityutskiy
2010-07-20 11:32 ` Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 13/16] writeback: restructure bdi forker loop a little Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 14/16] writeback: move bdi threads exiting logic to the forker thread Artem Bityutskiy
2010-07-18 7:02 ` Christoph Hellwig
2010-07-20 12:23 ` Artem Bityutskiy
2010-07-20 12:54 ` Artem Bityutskiy
2010-07-16 12:45 ` [RFC][PATCH 15/16] writeback: clean-up the warning about non-registered bdi Artem Bityutskiy
2010-07-18 7:03 ` Christoph Hellwig
2010-07-16 12:45 ` [RFC][PATCH 16/16] writeback: prevent unnecessary bdi threads wakeups Artem Bityutskiy
2010-07-18 7:45 ` Christoph Hellwig
2010-07-20 13:13 ` 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=1279284312-2411-13-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).