On 11/22/18 4:27 PM, Shenghui Wang wrote:
> "echo 1 > writeback_running" marks writeback_running even if no writeback
> kthread created as "d_strtoul(writeback_running)" will simply set dc->
> writeback_running without checking the existence of dc->writeback_thread.
>
> Add check for setting writeback_running via sysfs: if no writeback kthread
> available, reject setting to 1.
>
> Signed-off-by: Shenghui Wang <shhuiw@xxxxxxxxxxx>
> ---
> drivers/md/bcache/sysfs.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
> index 26f035a0c5b9..47f0f023ca05 100644
> --- a/drivers/md/bcache/sysfs.c
> +++ b/drivers/md/bcache/sysfs.c
> @@ -384,8 +384,25 @@ STORE(bch_cached_dev)
> mutex_lock(&bch_register_lock);
> size = __cached_dev_store(kobj, attr, buf, size);
>
> - if (attr == &sysfs_writeback_running)
> - bch_writeback_queue(dc);
> + if (attr == &sysfs_writeback_running) {
> + /* dc->writeback_running changed in __cached_dev_store() */
> + if (IS_ERR_OR_NULL(dc->writeback_thread)) {
> + /*
> + * reject setting it to 1 via sysfs if writeback
> + * kthread is not created yet.
> + */
> + if (dc->writeback_running) {
> + dc->writeback_running = false;
> + pr_err("%s: writeback thread not created yet",
> + dc->backing_dev_name);
Hi Shenghui,
May be the following information more clear,
"%s: failed to run non-existent writeback thread".
Also if you want to print out the device name, it should be the bcache
device name, I don't suggest to use backing device name.
The rested part of the patch is good to me :-)
Thanks.
Coly Li
> + }
> + } else
> + /*
> + * writeback kthread will check if dc->writeback_running
> + * is true or false.
> + */
> + bch_writeback_queue(dc);
> + }
>
> if (attr == &sysfs_writeback_percent)
> if (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags))
>
