On 11/5/19 10:56 PM, Song Liu wrote:
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"?
Naming is difficult to me :-), I can change it if you wish.
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?
Let me explain a little bit, the function are used in below cases:
1. write-behind mode is enabled and write-mostly device has multi queues,
the pool should be destroyed if the last device flagged with CollisionCheck
is removed from array, or the write-mostly flag will be cleared from the
last
device.
2. if we want to enable/disable serialization for normal raid1 in later
patch,
which means the pool could be created/destroyed whether bitmap is existed
or not, and all member device should be flagged with CollisionCheck.
The new argument is introduced to distinguish the two cases.
Thanks,
Guoqing
