[D/E,1/1] Revert "bcache: set CACHE_SET_IO_DISABLE in bch_cached_dev_error()"
diff mbox series

Message ID 20190708025131.15963-2-mfo@canonical.com
State New
Headers show
Series
  • LP#1829563 bcache: pending patch for Disco/Eoan
Related show

Commit Message

Mauricio Faria de Oliveira July 8, 2019, 2:51 a.m. UTC
From: Coly Li <colyli@suse.de>

BugLink: https://bugs.launchpad.net/bugs/1829563

This reverts commit 6147305c73e4511ca1a975b766b97a779d442567.

Although this patch helps the failed bcache device to stop faster when
too many I/O errors detected on corresponding cached device, setting
CACHE_SET_IO_DISABLE bit to cache set c->flags was not a good idea. This
operation will disable all I/Os on cache set, which means other attached
bcache devices won't work neither.

Without this patch, the failed bcache device can also be stopped
eventually if internal I/O accomplished (e.g. writeback). Therefore here
I revert it.

Fixes: 6147305c73e4 ("bcache: set CACHE_SET_IO_DISABLE in bch_cached_dev_error()")
Reported-by: Yong Li <mr.liyong@qq.com>
Signed-off-by: Coly Li <colyli@suse.de>
Cc: stable@vger.kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
(cherry picked from commit 695277f16b3a102fcc22c97fdf2de77c7b19f0b3 linux-next)
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 drivers/md/bcache/super.c | 17 -----------------
 1 file changed, 17 deletions(-)

Comments

Connor Kuehl July 16, 2019, 5:52 p.m. UTC | #1
On 7/7/19 7:51 PM, Mauricio Faria de Oliveira wrote:
> From: Coly Li <colyli@suse.de>
> 
> BugLink: https://bugs.launchpad.net/bugs/1829563
> 
> This reverts commit 6147305c73e4511ca1a975b766b97a779d442567.
> 
> Although this patch helps the failed bcache device to stop faster when
> too many I/O errors detected on corresponding cached device, setting
> CACHE_SET_IO_DISABLE bit to cache set c->flags was not a good idea. This
> operation will disable all I/Os on cache set, which means other attached
> bcache devices won't work neither.
> 
> Without this patch, the failed bcache device can also be stopped
> eventually if internal I/O accomplished (e.g. writeback). Therefore here
> I revert it.
> 
> Fixes: 6147305c73e4 ("bcache: set CACHE_SET_IO_DISABLE in bch_cached_dev_error()")
> Reported-by: Yong Li <mr.liyong@qq.com>
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (cherry picked from commit 695277f16b3a102fcc22c97fdf2de77c7b19f0b3 linux-next)
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>

Acked-by: Connor Kuehl <connor.kuehl@canonical.com>

> ---
>  drivers/md/bcache/super.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 169a959593e4..97db3daf3a77 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1436,8 +1436,6 @@ int bch_flash_dev_create(struct cache_set *c, uint64_t size)
>  
>  bool bch_cached_dev_error(struct cached_dev *dc)
>  {
> -	struct cache_set *c;
> -
>  	if (!dc || test_bit(BCACHE_DEV_CLOSING, &dc->disk.flags))
>  		return false;
>  
> @@ -1448,21 +1446,6 @@ bool bch_cached_dev_error(struct cached_dev *dc)
>  	pr_err("stop %s: too many IO errors on backing device %s\n",
>  		dc->disk.disk->disk_name, dc->backing_dev_name);
>  
> -	/*
> -	 * If the cached device is still attached to a cache set,
> -	 * even dc->io_disable is true and no more I/O requests
> -	 * accepted, cache device internal I/O (writeback scan or
> -	 * garbage collection) may still prevent bcache device from
> -	 * being stopped. So here CACHE_SET_IO_DISABLE should be
> -	 * set to c->flags too, to make the internal I/O to cache
> -	 * device rejected and stopped immediately.
> -	 * If c is NULL, that means the bcache device is not attached
> -	 * to any cache set, then no CACHE_SET_IO_DISABLE bit to set.
> -	 */
> -	c = dc->disk.c;
> -	if (c && test_and_set_bit(CACHE_SET_IO_DISABLE, &c->flags))
> -		pr_info("CACHE_SET_IO_DISABLE already set");
> -
>  	bcache_device_stop(&dc->disk);
>  	return true;
>  }
>

Patch
diff mbox series

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 169a959593e4..97db3daf3a77 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1436,8 +1436,6 @@  int bch_flash_dev_create(struct cache_set *c, uint64_t size)
 
 bool bch_cached_dev_error(struct cached_dev *dc)
 {
-	struct cache_set *c;
-
 	if (!dc || test_bit(BCACHE_DEV_CLOSING, &dc->disk.flags))
 		return false;
 
@@ -1448,21 +1446,6 @@  bool bch_cached_dev_error(struct cached_dev *dc)
 	pr_err("stop %s: too many IO errors on backing device %s\n",
 		dc->disk.disk->disk_name, dc->backing_dev_name);
 
-	/*
-	 * If the cached device is still attached to a cache set,
-	 * even dc->io_disable is true and no more I/O requests
-	 * accepted, cache device internal I/O (writeback scan or
-	 * garbage collection) may still prevent bcache device from
-	 * being stopped. So here CACHE_SET_IO_DISABLE should be
-	 * set to c->flags too, to make the internal I/O to cache
-	 * device rejected and stopped immediately.
-	 * If c is NULL, that means the bcache device is not attached
-	 * to any cache set, then no CACHE_SET_IO_DISABLE bit to set.
-	 */
-	c = dc->disk.c;
-	if (c && test_and_set_bit(CACHE_SET_IO_DISABLE, &c->flags))
-		pr_info("CACHE_SET_IO_DISABLE already set");
-
 	bcache_device_stop(&dc->disk);
 	return true;
 }