On Fri, Nov 1, 2019 at 7:23 AM <jgq516@xxxxxxxxx> wrote:
>
> From: Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx>
>
> The related resources (spin_lock, list and waitqueue) are needed
> for address raid1 reorder overlap issue too, so add "is_force"
> parameter to funcs (mddev_create/destroy_serial_pool).
I don't think is_force is a good name. Maybe just call it "force"?
>
> This parameter is set to true if we want to enable or disable
> raid1 io serialization by writinng sysfs node (in later patch),
> rdevs_init_serial is added and called when io serialization is
> enabled.
>
> Also rdev_init_serial and clear_bit(CollisionCheck, &rdev->flags)
> should be called between suspend and resume.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx>
> ---
> drivers/md/md-bitmap.c | 4 +--
> drivers/md/md.c | 64 ++++++++++++++++++++++++++++--------------
> drivers/md/md.h | 2 +-
> 3 files changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 5058716918ef..eff297cf5a81 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -1908,7 +1908,7 @@ int md_bitmap_load(struct mddev *mddev)
> goto out;
>
> rdev_for_each(rdev, mddev)
> - mddev_create_serial_pool(mddev, rdev, true);
> + mddev_create_serial_pool(mddev, rdev, true, false);
>
> if (mddev_is_clustered(mddev))
> md_cluster_ops->load_bitmaps(mddev, mddev->bitmap_info.nodes);
> @@ -2484,7 +2484,7 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> struct md_rdev *rdev;
>
> rdev_for_each(rdev, mddev)
> - mddev_create_serial_pool(mddev, rdev, false);
> + mddev_create_serial_pool(mddev, rdev, false, false);
> }
> if (old_mwb != backlog)
> md_bitmap_update_sb(mddev->bitmap);
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index bbf10b9301b6..43b7da334e4a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -127,9 +127,6 @@ static inline int speed_max(struct mddev *mddev)
>
> static int rdev_init_serial(struct md_rdev *rdev)
> {
> - if (rdev->bdev->bd_queue->nr_hw_queues == 1)
> - return 0;
> -
> spin_lock_init(&rdev->serial_list_lock);
> INIT_LIST_HEAD(&rdev->serial_list);
> init_waitqueue_head(&rdev->serial_io_wait);
> @@ -138,17 +135,27 @@ static int rdev_init_serial(struct md_rdev *rdev)
> return 1;
> }
>
> +static void rdevs_init_serial(struct mddev *mddev)
> +{
> + struct md_rdev *rdev;
> +
> + rdev_for_each(rdev, mddev)
> + rdev_init_serial(rdev);
> +}
> +
> /*
> - * Create serial_info_pool if rdev is the first multi-queue device flaged
> - * with writemostly, also write-behind mode is enabled.
> + * Create serial_info_pool for raid1 under conditions:
> + * 1. rdev is the first multi-queue device flaged with writemostly,
> + * also write-behind mode is enabled.
> + * 2. is_force is true which means we want to enable serialization
> + * for normal raid1 array.
> */
> void mddev_create_serial_pool(struct mddev *mddev, struct md_rdev *rdev,
> - bool is_suspend)
> + bool is_suspend, bool is_force)
> {
> - if (mddev->bitmap_info.max_write_behind == 0)
> - return;
> -
> - if (!test_bit(WriteMostly, &rdev->flags) || !rdev_init_serial(rdev))
> + if (!is_force && (mddev->bitmap_info.max_write_behind == 0 ||
> + (rdev && (rdev->bdev->bd_queue->nr_hw_queues == 1 ||
> + !test_bit(WriteMostly, &rdev->flags)))))
> return;
>
> if (mddev->serial_info_pool == NULL) {
> @@ -156,6 +163,10 @@ void mddev_create_serial_pool(struct mddev *mddev, struct md_rdev *rdev,
>
> if (!is_suspend)
> mddev_suspend(mddev);
> + if (is_force)
> + rdevs_init_serial(mddev);
> + if (!is_force && rdev)
> + rdev_init_serial(rdev);
> noio_flag = memalloc_noio_save();
> mddev->serial_info_pool =
> mempool_create_kmalloc_pool(NR_SERIAL_INFOS,
> @@ -171,11 +182,13 @@ EXPORT_SYMBOL_GPL(mddev_create_serial_pool);
>
> /*
> * Destroy serial_info_pool if rdev is the last device flaged with
> - * CollisionCheck.
> + * CollisionCheck, or is_force is true when we disable serialization
> + * for normal raid1.
> */
> -static void mddev_destroy_serial_pool(struct mddev *mddev, struct md_rdev *rdev)
> +static void mddev_destroy_serial_pool(struct mddev *mddev, struct md_rdev *rdev,
> + bool is_force)
I guess mddev_destroy_serial_pool() doesn't need an extra argument. We
can just to
clear_bit, etc.. no?
