On Mon, Oct 4, 2021 at 8:40 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> wrote:
>
> We shouldn't set it since write behind IO should only happen to write
> mostly device.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/md/md-bitmap.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index e29c6298ef5c..0346281b1555 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2469,11 +2469,28 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long backlog;
> unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
> + struct md_rdev *rdev;
> + bool has_write_mostly = false;
> int rv = kstrtoul(buf, 10, &backlog);
> if (rv)
> return rv;
> if (backlog > COUNTER_MAX)
> return -EINVAL;
> +
> + /*
> + * Without write mostly device, it doesn't make sense to set
> + * backlog for max_write_behind.
> + */
> + rdev_for_each(rdev, mddev)
> + if (test_bit(WriteMostly, &rdev->flags)) {
> + has_write_mostly = true;
> + break;
> + }
> + if (!has_write_mostly) {
> + pr_warn_ratelimited("md: No write mostly device available\n");
Most of these _store functions do not print warnings for invalid changes. So
I am not sure whether we want to add this one. If we do want it, we should
make it clear, as
"md: No write mostly device available. Cannot set backlog\n".
We may also add the device name there.
Thanks,
Song
> + return -EINVAL;
> + }
> +
> mddev->bitmap_info.max_write_behind = backlog;
> if (!backlog && mddev->serial_info_pool) {
> /* serial_info_pool is not needed if backlog is zero */
> --
> 2.31.1
>
