linux-bcache.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Coly Li <colyli@suse.de>
To: linux-bcache@vger.kernel.org
Cc: linux-block@vger.kernel.org, Coly Li <colyli@suse.de>,
	Michael Lyle <mlyle@lyle.org>, Hannes Reinecke <hare@suse.com>,
	Huijun Tang <tang.junhui@zte.com.cn>
Subject: [PATCH v3 05/13] bcache: quit dc->writeback_thread when BCACHE_DEV_DETACHING is set
Date: Sun, 14 Jan 2018 22:42:28 +0800	[thread overview]
Message-ID: <20180114144236.28213-6-colyli@suse.de> (raw)
In-Reply-To: <20180114144236.28213-1-colyli@suse.de>

In patch "bcache: fix cached_dev->count usage for bch_cache_set_error()",
cached_dev_get() is called when creating dc->writeback_thread, and
cached_dev_put() is called when exiting dc->writeback_thread. This
modification works well unless people detach the bcache device manually by
    'echo 1 > /sys/block/bcache<N>/bcache/detach'
Because this sysfs interface only calls bch_cached_dev_detach() which wakes
up dc->writeback_thread but does not stop it. The reason is, before patch
"bcache: fix cached_dev->count usage for bch_cache_set_error()", inside
bch_writeback_thread(), if cache is not dirty after writeback,
cached_dev_put() will be called here. And in cached_dev_make_request() when
a new write request makes cache from clean to dirty, cached_dev_get() will
be called there. Since we don't operate dc->count in these locations,
refcount d->count cannot be dropped after cache becomes clean, and
cached_dev_detach_finish() won't be called to detach bcache device.

This patch fixes the issue by checking whether BCACHE_DEV_DETACHING is
set inside bch_writeback_thread(). If this bit is set and cache is clean
(no existing writeback_keys), break the while-loop, call cached_dev_put()
and quit the writeback thread.

Please note if cache is still dirty, even BCACHE_DEV_DETACHING is set the
writeback thread should continue to perform writeback, this is the original
design of manually detach.

I compose a separte patch because that patch "bcache: fix cached_dev->count
usage for bch_cache_set_error()" already gets a "Reviewed-by:" from Hannes
Reinecke. Also this fix is not trivial and good for a separate patch.

Signed-off-by: Coly Li <colyli@suse.de>
Cc: Michael Lyle <mlyle@lyle.org>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Huijun Tang <tang.junhui@zte.com.cn>
---
 drivers/md/bcache/writeback.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
index b280c134dd4d..4dbeaaa575bf 100644
--- a/drivers/md/bcache/writeback.c
+++ b/drivers/md/bcache/writeback.c
@@ -565,9 +565,15 @@ static int bch_writeback_thread(void *arg)
 	while (!kthread_should_stop()) {
 		down_write(&dc->writeback_lock);
 		set_current_state(TASK_INTERRUPTIBLE);
-		if (!atomic_read(&dc->has_dirty) ||
-		    (!test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags) &&
-		     !dc->writeback_running)) {
+		/*
+		 * If the bache device is detaching, skip here and continue
+		 * to perform writeback. Otherwise, if no dirty data on cache,
+		 * or there is dirty data on cache but writeback is disabled,
+		 * the writeback thread should sleep here and wait for others
+		 * to wake up it.
+		 */
+		if (!test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags) &&
+		    (!atomic_read(&dc->has_dirty) || !dc->writeback_running)) {
 			up_write(&dc->writeback_lock);
 
 			if (kthread_should_stop()) {
@@ -587,6 +593,14 @@ static int bch_writeback_thread(void *arg)
 			atomic_set(&dc->has_dirty, 0);
 			SET_BDEV_STATE(&dc->sb, BDEV_STATE_CLEAN);
 			bch_write_bdev_super(dc, NULL);
+			/*
+			 * If bcache device is detaching via sysfs interface,
+			 * writeback thread should stop after there is no dirty
+			 * data on cache. BCACHE_DEV_DETACHING flag is set in
+			 * bch_cached_dev_detach().
+			 */
+			if (test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags))
+				break;
 		}
 
 		up_write(&dc->writeback_lock);
-- 
2.15.1

  parent reply	other threads:[~2018-01-14 14:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-14 14:42 [PATCH v3 00/13] bcache: device failure handling improvement Coly Li
2018-01-14 14:42 ` [PATCH v3 01/13] bcache: set writeback_rate_update_seconds in range [1, 60] seconds Coly Li
2018-01-16  9:03   ` Hannes Reinecke
2018-01-14 14:42 ` [PATCH v3 02/13] bcache: properly set task state in bch_writeback_thread() Coly Li
2018-01-16  9:02   ` Hannes Reinecke
2018-01-14 14:42 ` [PATCH v3 03/13] bcache: set task properly in allocator_wait() Coly Li
2018-01-16  9:05   ` Hannes Reinecke
2018-01-16  9:29     ` Coly Li
2018-01-14 14:42 ` [PATCH v3 04/13] bcache: fix cached_dev->count usage for bch_cache_set_error() Coly Li
2018-01-14 14:42 ` Coly Li [this message]
2018-01-16  9:11   ` [PATCH v3 05/13] bcache: quit dc->writeback_thread when BCACHE_DEV_DETACHING is set Hannes Reinecke
2018-01-26  6:21     ` Coly Li
2018-01-14 14:42 ` [PATCH v3 06/13] bcache: stop dc->writeback_rate_update properly Coly Li
2018-01-14 14:42 ` [PATCH v3 07/13] bcache: set error_limit correctly Coly Li
2018-01-14 14:42 ` [PATCH v3 08/13] bcache: add CACHE_SET_IO_DISABLE to struct cache_set flags Coly Li
2018-01-14 14:42 ` [PATCH v3 09/13] bcache: stop all attached bcache devices for a retired cache set Coly Li
2018-01-14 14:42 ` [PATCH v3 10/13] bcache: fix inaccurate io state for detached bcache devices Coly Li
2018-01-16  9:27   ` Hannes Reinecke
2018-01-14 14:42 ` [PATCH v3 11/13] bcache: add backing_request_endio() for bi_end_io of attached backing device I/O Coly Li
2018-01-16  9:28   ` Hannes Reinecke
2018-01-14 14:42 ` [PATCH v3 12/13] bcache: add io_disable to struct cached_dev Coly Li
2018-01-16  9:32   ` Hannes Reinecke
2018-01-14 14:42 ` [PATCH v3 13/13] bcache: stop bcache device when backing device is offline Coly Li
2018-01-16  9:33   ` Hannes Reinecke
2018-01-24 22:23 ` [PATCH v3 00/13] bcache: device failure handling improvement Nix
2018-01-25  3:35   ` Re[2]: " Pavel Goran
2018-01-25 18:57     ` Nix
2018-01-26  4:15       ` Re[2]: " Pavel Goran
2018-01-26  4:56         ` Coly Li
2018-01-26  5:51           ` Michael Lyle
2018-01-26  6:23             ` Coly Li
2018-02-16 12:11           ` Nix

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=20180114144236.28213-6-colyli@suse.de \
    --to=colyli@suse.de \
    --cc=hare@suse.com \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=mlyle@lyle.org \
    --cc=tang.junhui@zte.com.cn \
    /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).