linux-bcache.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] bcache: more fixes for v5.8-rc1
@ 2020-06-14 16:53 colyli
  2020-06-14 16:53 ` colyli
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li

From: Coly Li <colyli@suse.de>

Hi Jens,

Here are more following up fixes for bcache v5.8-rc1.

The two patches from me are minor clean up. Rested two patches
are important.

- Mauricio Faria de Oliveira contributes a fix for a potential
  kernel panic when users configures improper block size value
  to bcache backing device. This problem should be fixed as soon
  as possible IMHO.
- Zhiqiang Liu contributes a fix for a potential deadlock (even quite
  hard to trigger), which I want to have it as soon as possible.

Please take these patches for v5.8-rc1, or -rc2 if it is late for -rc1.

Thanks you in advance.

Coly Li
---

Coly Li (2):
  bcache: use delayed kworker fo asynchronous devices registration
  bcache: pr_info() format clean up in bcache_device_init()

Mauricio Faria de Oliveira (1):
  bcache: check and adjust logical block size for backing devices

Zhiqiang Liu (1):
  bcache: fix potential deadlock problem in btree_gc_coalesce

 drivers/md/bcache/btree.c |  8 ++++++--
 drivers/md/bcache/super.c | 35 ++++++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 11 deletions(-)

-- 
2.25.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 0/4] bcache: more fixes for v5.8-rc1
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
@ 2020-06-14 16:53 ` colyli
  2020-06-14 16:53 ` [PATCH 1/4] bcache: fix potential deadlock problem in btree_gc_coalesce colyli
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li

From: Coly Li <colyli@suse.de>

Hi Jens,

Here are more following up fixes for bcache v5.8-rc1.

The two patches from me are minor clean up. Rested two patches
are important.

- Mauricio Faria de Oliveira contributes a fix for a potential
  kernel panic when users configures improper block size value
  to bcache backing device. This problem should be fixed as soon
  as possible IMHO.
- Zhiqiang Liu contributes a fix for a potential deadlock (even quite
  hard to trigger), which I want to have it as soon as possible.

Please take these patches for v5.8-rc1, or -rc2 if it is late for -rc1.

Thanks you in advance.

Coly Li
---

Coly Li (2):
  bcache: use delayed kworker fo asynchronous devices registration
  bcache: pr_info() format clean up in bcache_device_init()

Mauricio Faria de Oliveira (1):
  bcache: check and adjust logical block size for backing devices

Zhiqiang Liu (1):
  bcache: fix potential deadlock problem in btree_gc_coalesce

 drivers/md/bcache/btree.c |  8 ++++++--
 drivers/md/bcache/super.c | 35 ++++++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 11 deletions(-)

-- 
2.25.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] bcache: fix potential deadlock problem in btree_gc_coalesce
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
  2020-06-14 16:53 ` colyli
@ 2020-06-14 16:53 ` colyli
  2020-06-14 16:53 ` [PATCH 2/4] bcache: check and adjust logical block size for backing devices colyli
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Zhiqiang Liu, Coly Li

From: Zhiqiang Liu <liuzhiqiang26@huawei.com>

coccicheck reports:
  drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417

In btree_gc_coalesce func, if the coalescing process fails, we will goto
to out_nocoalesce tag directly without releasing new_nodes[i]->write_lock.
Then, it will cause a deadlock when trying to acquire new_nodes[i]->
write_lock for freeing new_nodes[i] before return.

btree_gc_coalesce func details as follows:
	if alloc new_nodes[i] fails:
		goto out_nocoalesce;
	// obtain new_nodes[i]->write_lock
	mutex_lock(&new_nodes[i]->write_lock)
	// main coalescing process
	for (i = nodes - 1; i > 0; --i)
		[snipped]
		if coalescing process fails:
			// Here, directly goto out_nocoalesce
			 // tag will cause a deadlock
			goto out_nocoalesce;
		[snipped]
	// release new_nodes[i]->write_lock
	mutex_unlock(&new_nodes[i]->write_lock)
	// coalesing succ, return
	return;
out_nocoalesce:
	btree_node_free(new_nodes[i])	// free new_nodes[i]
	// obtain new_nodes[i]->write_lock
	mutex_lock(&new_nodes[i]->write_lock);
	// set flag for reuse
	clear_bit(BTREE_NODE_dirty, &ew_nodes[i]->flags);
	// release new_nodes[i]->write_lock
	mutex_unlock(&new_nodes[i]->write_lock);

To fix the problem, we add a new tag 'out_unlock_nocoalesce' for
releasing new_nodes[i]->write_lock before out_nocoalesce tag. If
coalescing process fails, we will go to out_unlock_nocoalesce tag
for releasing new_nodes[i]->write_lock before free new_nodes[i] in
out_nocoalesce tag.

(Coly Li helps to clean up commit log format.)

Fixes: 2a285686c109816 ("bcache: btree locking rework")
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/btree.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 39de94edd73a..6548a601edf0 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1389,7 +1389,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
 			if (__set_blocks(n1, n1->keys + n2->keys,
 					 block_bytes(b->c)) >
 			    btree_blocks(new_nodes[i]))
-				goto out_nocoalesce;
+				goto out_unlock_nocoalesce;
 
 			keys = n2->keys;
 			/* Take the key of the node we're getting rid of */
@@ -1418,7 +1418,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
 
 		if (__bch_keylist_realloc(&keylist,
 					  bkey_u64s(&new_nodes[i]->key)))
-			goto out_nocoalesce;
+			goto out_unlock_nocoalesce;
 
 		bch_btree_node_write(new_nodes[i], &cl);
 		bch_keylist_add(&keylist, &new_nodes[i]->key);
@@ -1464,6 +1464,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
 	/* Invalidated our iterator */
 	return -EINTR;
 
+out_unlock_nocoalesce:
+	for (i = 0; i < nodes; i++)
+		mutex_unlock(&new_nodes[i]->write_lock);
+
 out_nocoalesce:
 	closure_sync(&cl);
 
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] bcache: check and adjust logical block size for backing devices
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
  2020-06-14 16:53 ` colyli
  2020-06-14 16:53 ` [PATCH 1/4] bcache: fix potential deadlock problem in btree_gc_coalesce colyli
@ 2020-06-14 16:53 ` colyli
  2020-06-14 16:53 ` [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration colyli
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe
  Cc: linux-bcache, linux-block, Mauricio Faria de Oliveira,
	Ryan Finnie, Sebastian Marsching, Coly Li

From: Mauricio Faria de Oliveira <mfo@canonical.com>

It's possible for a block driver to set logical block size to
a value greater than page size incorrectly; e.g. bcache takes
the value from the superblock, set by the user w/ make-bcache.

This causes a BUG/NULL pointer dereference in the path:

  __blkdev_get()
  -> set_init_blocksize() // set i_blkbits based on ...
     -> bdev_logical_block_size()
        -> queue_logical_block_size() // ... this value
  -> bdev_disk_changed()
     ...
     -> blkdev_readpage()
        -> block_read_full_page()
           -> create_page_buffers() // size = 1 << i_blkbits
              -> create_empty_buffers() // give size/take pointer
                 -> alloc_page_buffers() // return NULL
                 .. BUG!

Because alloc_page_buffers() is called with size > PAGE_SIZE,
thus it initializes head = NULL, skips the loop, return head;
then create_empty_buffers() gets (and uses) the NULL pointer.

This has been around longer than commit ad6bf88a6c19 ("block:
fix an integer overflow in logical block size"); however, it
increased the range of values that can trigger the issue.

Previously only 8k/16k/32k (on x86/4k page size) would do it,
as greater values overflow unsigned short to zero, and queue_
logical_block_size() would then use the default of 512.

Now the range with unsigned int is much larger, and users w/
the 512k value, which happened to be zero'ed previously and
work fine, started to hit this issue -- as the zero is gone,
and queue_logical_block_size() does return 512k (>PAGE_SIZE.)

Fix this by checking the bcache device's logical block size,
and if it's greater than page size, fallback to the backing/
cached device's logical page size.

This doesn't affect cache devices as those are still checked
for block/page size in read_super(); only the backing/cached
devices are not.

Apparently it's a regression from commit 2903381fce71 ("bcache:
Take data offset from the bdev superblock."), moving the check
into BCACHE_SB_VERSION_CDEV only. Now that we have superblocks
of backing devices out there with this larger value, we cannot
refuse to load them (i.e., have a similar check in _BDEV.)

Ideally perhaps bcache should use all values from the backing
device (physical/logical/io_min block size)? But for now just
fix the problematic case.

Test-case:

    # IMG=/root/disk.img
    # dd if=/dev/zero of=$IMG bs=1 count=0 seek=1G
    # DEV=$(losetup --find --show $IMG)
    # make-bcache --bdev $DEV --block 8k
      < see dmesg >

Before:

    # uname -r
    5.7.0-rc7

    [   55.944046] BUG: kernel NULL pointer dereference, address: 0000000000000000
    ...
    [   55.949742] CPU: 3 PID: 610 Comm: bcache-register Not tainted 5.7.0-rc7 #4
    ...
    [   55.952281] RIP: 0010:create_empty_buffers+0x1a/0x100
    ...
    [   55.966434] Call Trace:
    [   55.967021]  create_page_buffers+0x48/0x50
    [   55.967834]  block_read_full_page+0x49/0x380
    [   55.972181]  do_read_cache_page+0x494/0x610
    [   55.974780]  read_part_sector+0x2d/0xaa
    [   55.975558]  read_lba+0x10e/0x1e0
    [   55.977904]  efi_partition+0x120/0x5a6
    [   55.980227]  blk_add_partitions+0x161/0x390
    [   55.982177]  bdev_disk_changed+0x61/0xd0
    [   55.982961]  __blkdev_get+0x350/0x490
    [   55.983715]  __device_add_disk+0x318/0x480
    [   55.984539]  bch_cached_dev_run+0xc5/0x270
    [   55.986010]  register_bcache.cold+0x122/0x179
    [   55.987628]  kernfs_fop_write+0xbc/0x1a0
    [   55.988416]  vfs_write+0xb1/0x1a0
    [   55.989134]  ksys_write+0x5a/0xd0
    [   55.989825]  do_syscall_64+0x43/0x140
    [   55.990563]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
    [   55.991519] RIP: 0033:0x7f7d60ba3154
    ...

After:

    # uname -r
    5.7.0.bcachelbspgsz

    [   31.672460] bcache: bcache_device_init() bcache0: sb/logical block size (8192) greater than page size (4096) falling back to device logical block size (512)
    [   31.675133] bcache: register_bdev() registered backing device loop0

    # grep ^ /sys/block/bcache0/queue/*_block_size
    /sys/block/bcache0/queue/logical_block_size:512
    /sys/block/bcache0/queue/physical_block_size:8192

Reported-by: Ryan Finnie <ryan@finnie.org>
Reported-by: Sebastian Marsching <sebastian@marsching.com>
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Signed-off-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/super.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f9975c22bf7e..904ea9bbb5c4 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -819,7 +819,8 @@ static void bcache_device_free(struct bcache_device *d)
 }
 
 static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
-			      sector_t sectors, make_request_fn make_request_fn)
+			      sector_t sectors, make_request_fn make_request_fn,
+			      struct block_device *cached_bdev)
 {
 	struct request_queue *q;
 	const size_t max_stripes = min_t(size_t, INT_MAX,
@@ -885,6 +886,21 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 	q->limits.io_min		= block_size;
 	q->limits.logical_block_size	= block_size;
 	q->limits.physical_block_size	= block_size;
+
+	if (q->limits.logical_block_size > PAGE_SIZE && cached_bdev) {
+		/*
+		 * This should only happen with BCACHE_SB_VERSION_BDEV.
+		 * Block/page size is checked for BCACHE_SB_VERSION_CDEV.
+		 */
+		pr_info("%s: sb/logical block size (%u) greater than page size "
+			"(%lu) falling back to device logical block size (%u)",
+			d->disk->disk_name, q->limits.logical_block_size,
+			PAGE_SIZE, bdev_logical_block_size(cached_bdev));
+
+		/* This also adjusts physical block size/min io size if needed */
+		blk_queue_logical_block_size(q, bdev_logical_block_size(cached_bdev));
+	}
+
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, d->disk->queue);
 	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, d->disk->queue);
 	blk_queue_flag_set(QUEUE_FLAG_DISCARD, d->disk->queue);
@@ -1340,7 +1356,7 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
 
 	ret = bcache_device_init(&dc->disk, block_size,
 			 dc->bdev->bd_part->nr_sects - dc->sb.data_offset,
-			 cached_dev_make_request);
+			 cached_dev_make_request, dc->bdev);
 	if (ret)
 		return ret;
 
@@ -1453,7 +1469,7 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
 	kobject_init(&d->kobj, &bch_flash_dev_ktype);
 
 	if (bcache_device_init(d, block_bytes(c), u->sectors,
-			flash_dev_make_request))
+			flash_dev_make_request, NULL))
 		goto err;
 
 	bcache_device_attach(d, c, u - c->uuids);
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
                   ` (2 preceding siblings ...)
  2020-06-14 16:53 ` [PATCH 2/4] bcache: check and adjust logical block size for backing devices colyli
@ 2020-06-14 16:53 ` colyli
  2020-06-14 16:53   ` colyli
  2020-06-15  6:12   ` Hannes Reinecke
  2020-06-14 16:53 ` [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init() colyli
  2020-06-14 22:48 ` [PATCH 0/4] bcache: more fixes for v5.8-rc1 Jens Axboe
  5 siblings, 2 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li, Hannes Reinecke

From: Coly Li <colyli@suse.de>

This patch changes the asynchronous registration kworker to a delayed
kworker. There is probability queue_work() queues the async registration
kworker to the same CPU (even though very little), then the process
which writing sysfs interface to reigster bcache device may won't return
immeidately. queue_delayed_work() in this patch will delay 10 jiffies
before insert the kworker to run queue, which makes sure the registering
process may always returns to user space in time.

Fixes: 9e23ccf8f0a22 ("bcache: asynchronous devices registration")
Signed-off-by: Coly Li <colyli@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
---
 drivers/md/bcache/super.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 904ea9bbb5c4..a6e79083010a 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -19,6 +19,7 @@
 #include <linux/genhd.h>
 #include <linux/idr.h>
 #include <linux/kthread.h>
+#include <linux/workqueue.h>
 #include <linux/module.h>
 #include <linux/random.h>
 #include <linux/reboot.h>
@@ -2380,7 +2381,7 @@ static bool bch_is_open(struct block_device *bdev)
 }
 
 struct async_reg_args {
-	struct work_struct reg_work;
+	struct delayed_work reg_work;
 	char *path;
 	struct cache_sb *sb;
 	struct cache_sb_disk *sb_disk;
@@ -2391,7 +2392,7 @@ static void register_bdev_worker(struct work_struct *work)
 {
 	int fail = false;
 	struct async_reg_args *args =
-		container_of(work, struct async_reg_args, reg_work);
+		container_of(work, struct async_reg_args, reg_work.work);
 	struct cached_dev *dc;
 
 	dc = kzalloc(sizeof(*dc), GFP_KERNEL);
@@ -2421,7 +2422,7 @@ static void register_cache_worker(struct work_struct *work)
 {
 	int fail = false;
 	struct async_reg_args *args =
-		container_of(work, struct async_reg_args, reg_work);
+		container_of(work, struct async_reg_args, reg_work.work);
 	struct cache *ca;
 
 	ca = kzalloc(sizeof(*ca), GFP_KERNEL);
@@ -2449,11 +2450,12 @@ static void register_cache_worker(struct work_struct *work)
 static void register_device_aync(struct async_reg_args *args)
 {
 	if (SB_IS_BDEV(args->sb))
-		INIT_WORK(&args->reg_work, register_bdev_worker);
+		INIT_DELAYED_WORK(&args->reg_work, register_bdev_worker);
 	else
-		INIT_WORK(&args->reg_work, register_cache_worker);
+		INIT_DELAYED_WORK(&args->reg_work, register_cache_worker);
 
-	queue_work(system_wq, &args->reg_work);
+	/* 10 jiffies is enough for a delay */
+	queue_delayed_work(system_wq, &args->reg_work, 10);
 }
 
 static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration
  2020-06-14 16:53 ` [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration colyli
@ 2020-06-14 16:53   ` colyli
  2020-06-15  6:12   ` Hannes Reinecke
  1 sibling, 0 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li, Hannes Reinecke

From: Coly Li <colyli@suse.de>

This patch changes the asynchronous registration kworker to a delayed
kworker. There is probability queue_work() queues the async registration
kworker to the same CPU (even though very little), then the process
which writing sysfs interface to reigster bcache device may won't return
immeidately. queue_delayed_work() in this patch will delay 10 jiffies
before insert the kworker to run queue, which makes sure the registering
process may always returns to user space in time.

Fixes: 9e23ccf8f0a22 ("bcache: asynchronous devices registration")
Signed-off-by: Coly Li <colyli@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
---
 drivers/md/bcache/super.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 904ea9bbb5c4..a6e79083010a 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -19,6 +19,7 @@
 #include <linux/genhd.h>
 #include <linux/idr.h>
 #include <linux/kthread.h>
+#include <linux/workqueue.h>
 #include <linux/module.h>
 #include <linux/random.h>
 #include <linux/reboot.h>
@@ -2380,7 +2381,7 @@ static bool bch_is_open(struct block_device *bdev)
 }
 
 struct async_reg_args {
-	struct work_struct reg_work;
+	struct delayed_work reg_work;
 	char *path;
 	struct cache_sb *sb;
 	struct cache_sb_disk *sb_disk;
@@ -2391,7 +2392,7 @@ static void register_bdev_worker(struct work_struct *work)
 {
 	int fail = false;
 	struct async_reg_args *args =
-		container_of(work, struct async_reg_args, reg_work);
+		container_of(work, struct async_reg_args, reg_work.work);
 	struct cached_dev *dc;
 
 	dc = kzalloc(sizeof(*dc), GFP_KERNEL);
@@ -2421,7 +2422,7 @@ static void register_cache_worker(struct work_struct *work)
 {
 	int fail = false;
 	struct async_reg_args *args =
-		container_of(work, struct async_reg_args, reg_work);
+		container_of(work, struct async_reg_args, reg_work.work);
 	struct cache *ca;
 
 	ca = kzalloc(sizeof(*ca), GFP_KERNEL);
@@ -2449,11 +2450,12 @@ static void register_cache_worker(struct work_struct *work)
 static void register_device_aync(struct async_reg_args *args)
 {
 	if (SB_IS_BDEV(args->sb))
-		INIT_WORK(&args->reg_work, register_bdev_worker);
+		INIT_DELAYED_WORK(&args->reg_work, register_bdev_worker);
 	else
-		INIT_WORK(&args->reg_work, register_cache_worker);
+		INIT_DELAYED_WORK(&args->reg_work, register_cache_worker);
 
-	queue_work(system_wq, &args->reg_work);
+	/* 10 jiffies is enough for a delay */
+	queue_delayed_work(system_wq, &args->reg_work, 10);
 }
 
 static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init()
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
                   ` (3 preceding siblings ...)
  2020-06-14 16:53 ` [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration colyli
@ 2020-06-14 16:53 ` colyli
  2020-06-14 16:53   ` colyli
  2020-06-14 22:48 ` [PATCH 0/4] bcache: more fixes for v5.8-rc1 Jens Axboe
  5 siblings, 1 reply; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li

From: Coly Li <colyli@suse.de>

scripts/checkpatch.pl reports following warning for patch
("bcache: check and adjust logical block size for backing devices"),
    WARNING: quoted string split across lines
    #146: FILE: drivers/md/bcache/super.c:896:
    +  pr_info("%s: sb/logical block size (%u) greater than page size "
    +	       "(%lu) falling back to device logical block size (%u)",

There are two things to fix up,
- The kernel message print should be in a single line.
- pr_info() won't automatically add new line since v5.8, a '\n' should
  be added.

This patch just does the above cleanup in bcache_device_init().

Signed-off-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index a6e79083010a..2014016f9a60 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -893,8 +893,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 		 * This should only happen with BCACHE_SB_VERSION_BDEV.
 		 * Block/page size is checked for BCACHE_SB_VERSION_CDEV.
 		 */
-		pr_info("%s: sb/logical block size (%u) greater than page size "
-			"(%lu) falling back to device logical block size (%u)",
+		pr_info("%s: sb/logical block size (%u) greater than page size (%lu) falling back to device logical block size (%u)\n",
 			d->disk->disk_name, q->limits.logical_block_size,
 			PAGE_SIZE, bdev_logical_block_size(cached_bdev));
 
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init()
  2020-06-14 16:53 ` [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init() colyli
@ 2020-06-14 16:53   ` colyli
  0 siblings, 0 replies; 11+ messages in thread
From: colyli @ 2020-06-14 16:53 UTC (permalink / raw)
  To: axboe; +Cc: linux-bcache, linux-block, Coly Li

From: Coly Li <colyli@suse.de>

scripts/checkpatch.pl reports following warning for patch
("bcache: check and adjust logical block size for backing devices"),
    WARNING: quoted string split across lines
    #146: FILE: drivers/md/bcache/super.c:896:
    +  pr_info("%s: sb/logical block size (%u) greater than page size "
    +	       "(%lu) falling back to device logical block size (%u)",

There are two things to fix up,
- The kernel message print should be in a single line.
- pr_info() won't automatically add new line since v5.8, a '\n' should
  be added.

This patch just does the above cleanup in bcache_device_init().

Signed-off-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index a6e79083010a..2014016f9a60 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -893,8 +893,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 		 * This should only happen with BCACHE_SB_VERSION_BDEV.
 		 * Block/page size is checked for BCACHE_SB_VERSION_CDEV.
 		 */
-		pr_info("%s: sb/logical block size (%u) greater than page size "
-			"(%lu) falling back to device logical block size (%u)",
+		pr_info("%s: sb/logical block size (%u) greater than page size (%lu) falling back to device logical block size (%u)\n",
 			d->disk->disk_name, q->limits.logical_block_size,
 			PAGE_SIZE, bdev_logical_block_size(cached_bdev));
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] bcache: more fixes for v5.8-rc1
  2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
                   ` (4 preceding siblings ...)
  2020-06-14 16:53 ` [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init() colyli
@ 2020-06-14 22:48 ` Jens Axboe
  5 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2020-06-14 22:48 UTC (permalink / raw)
  To: colyli; +Cc: linux-bcache, linux-block

On 6/14/20 10:53 AM, colyli@suse.de wrote:
> From: Coly Li <colyli@suse.de>
> 
> Hi Jens,
> 
> Here are more following up fixes for bcache v5.8-rc1.
> 
> The two patches from me are minor clean up. Rested two patches
> are important.
> 
> - Mauricio Faria de Oliveira contributes a fix for a potential
>   kernel panic when users configures improper block size value
>   to bcache backing device. This problem should be fixed as soon
>   as possible IMHO.
> - Zhiqiang Liu contributes a fix for a potential deadlock (even quite
>   hard to trigger), which I want to have it as soon as possible.
> 
> Please take these patches for v5.8-rc1, or -rc2 if it is late for -rc1.

A few hours before -rc1 release is indeed too late. I'd only queue and
push anything this late if there was a very compelling reason to do so.

I've queued them up for -rc2.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration
  2020-06-14 16:53 ` [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration colyli
  2020-06-14 16:53   ` colyli
@ 2020-06-15  6:12   ` Hannes Reinecke
  2020-06-15  6:12     ` Hannes Reinecke
  1 sibling, 1 reply; 11+ messages in thread
From: Hannes Reinecke @ 2020-06-15  6:12 UTC (permalink / raw)
  To: colyli, axboe; +Cc: linux-bcache, linux-block

On 6/14/20 6:53 PM, colyli@suse.de wrote:
> From: Coly Li <colyli@suse.de>
> 
> This patch changes the asynchronous registration kworker to a delayed
> kworker. There is probability queue_work() queues the async registration
> kworker to the same CPU (even though very little), then the process
> which writing sysfs interface to reigster bcache device may won't return
> immeidately. queue_delayed_work() in this patch will delay 10 jiffies
> before insert the kworker to run queue, which makes sure the registering
> process may always returns to user space in time.
> 
> Fixes: 9e23ccf8f0a22 ("bcache: asynchronous devices registration")
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/md/bcache/super.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration
  2020-06-15  6:12   ` Hannes Reinecke
@ 2020-06-15  6:12     ` Hannes Reinecke
  0 siblings, 0 replies; 11+ messages in thread
From: Hannes Reinecke @ 2020-06-15  6:12 UTC (permalink / raw)
  To: colyli, axboe; +Cc: linux-bcache, linux-block

On 6/14/20 6:53 PM, colyli@suse.de wrote:
> From: Coly Li <colyli@suse.de>
> 
> This patch changes the asynchronous registration kworker to a delayed
> kworker. There is probability queue_work() queues the async registration
> kworker to the same CPU (even though very little), then the process
> which writing sysfs interface to reigster bcache device may won't return
> immeidately. queue_delayed_work() in this patch will delay 10 jiffies
> before insert the kworker to run queue, which makes sure the registering
> process may always returns to user space in time.
> 
> Fixes: 9e23ccf8f0a22 ("bcache: asynchronous devices registration")
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/md/bcache/super.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-15  6:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-14 16:53 [PATCH 0/4] bcache: more fixes for v5.8-rc1 colyli
2020-06-14 16:53 ` colyli
2020-06-14 16:53 ` [PATCH 1/4] bcache: fix potential deadlock problem in btree_gc_coalesce colyli
2020-06-14 16:53 ` [PATCH 2/4] bcache: check and adjust logical block size for backing devices colyli
2020-06-14 16:53 ` [PATCH 3/4] bcache: use delayed kworker fo asynchronous devices registration colyli
2020-06-14 16:53   ` colyli
2020-06-15  6:12   ` Hannes Reinecke
2020-06-15  6:12     ` Hannes Reinecke
2020-06-14 16:53 ` [PATCH 4/4] bcache: pr_info() format clean up in bcache_device_init() colyli
2020-06-14 16:53   ` colyli
2020-06-14 22:48 ` [PATCH 0/4] bcache: more fixes for v5.8-rc1 Jens Axboe

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).