[PATCH 0/3 v3] add resync speed control for dm-raid1

[PATCH 0/3 v3] add resync speed control for dm-raid1

[Guangliang Zhao]

Hi,

These patches are used to add resync speed control for dm-raid1. The
second and third patch provide support for user-space tool dmsetup.
I have made some modifications by the comments. This is the third
version.

Guangliang Zhao (3):
  dm raid1: add resync speed control for dm-raid1
  dm raid1: add interface to set resync speed
  dm raid1: add interface to get resync speed

 drivers/md/dm-raid1.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 1 deletion(-)

-- 
1.7.10.4

[PATCH 1/3 v3] dm raid1: add resync speed control for dm-raid1

[Guangliang Zhao]

The IO performance on the already available lv is very bad
during the initial sync when we create a mirror lv. This
patch add the rate limit for every mirror target to control
resync speed.

This patch only limited the resync IO speed simply, and
didn't care about user IO, because it is mainly used in
clustered situations, and we don't know the IO load on
other nodes, so it isn't practical to measure them in
those cases.

Signed-off-by: Guangliang Zhao <gzhao@suse.com>
---
 drivers/md/dm-raid1.c |   30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 9bfd057..43e428a 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -13,6 +13,7 @@
 #include <linux/pagemap.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <linux/ratelimit.h>
 #include <linux/device-mapper.h>
 #include <linux/dm-io.h>
 #include <linux/dm-dirty-log.h>
@@ -26,6 +27,11 @@
 #define DM_RAID1_HANDLE_ERRORS 0x01
 #define errors_handled(p)	((p)->features & DM_RAID1_HANDLE_ERRORS)
 
+/* Default resync interval*/
+#define RESYNC_JIFFIES	HZ
+/* Default max resync speed in KB/s */
+#define DEFAULT_RESYNC_SPEED		(0xFFFFFFF)
+
 static DECLARE_WAIT_QUEUE_HEAD(_kmirrord_recovery_stopped);
 
 /*-----------------------------------------------------------------
@@ -69,6 +75,9 @@ struct mirror_set {
 	int log_failure;
 	int leg_failure;
 	atomic_t suspend;
+	/* recovery speed control */
+	struct ratelimit_state ms_rlimit;
+	struct delayed_work resync_waker;
 
 	atomic_t default_mirror;	/* Default mirror */
 
@@ -383,12 +392,18 @@ static void do_recovery(struct mirror_set *ms)
 	/*
 	 * Copy any already quiesced regions.
 	 */
-	while ((reg = dm_rh_recovery_start(ms->rh))) {
+	while (__ratelimit(&ms->ms_rlimit) &&
+			(reg = dm_rh_recovery_start(ms->rh))) {
 		r = recover(ms, reg);
 		if (r)
 			dm_rh_recovery_end(reg, 0);
 	}
 
+	/* Broke off the resync process, so wake up kmirrord later on */
+	if (!ms->in_sync)
+		schedule_delayed_work(&ms->resync_waker,
+				      ms->ms_rlimit.interval);
+
 	/*
 	 * Update the in sync flag.
 	 */
@@ -842,6 +857,14 @@ static void do_mirror(struct work_struct *work)
 	do_failures(ms, &failures);
 }
 
+static void do_resync_wake(struct work_struct *work)
+{
+	struct mirror_set *ms =
+		container_of(work, struct mirror_set, resync_waker.work);
+
+	wakeup_mirrord(ms);
+}
+
 /*-----------------------------------------------------------------
  * Target functions
  *---------------------------------------------------------------*/
@@ -852,6 +875,7 @@ static struct mirror_set *alloc_context(unsigned int nr_mirrors,
 {
 	size_t len;
 	struct mirror_set *ms = NULL;
+	int burst;
 
 	len = sizeof(*ms) + (sizeof(ms->mirror[0]) * nr_mirrors);
 
@@ -875,6 +899,10 @@ static struct mirror_set *alloc_context(unsigned int nr_mirrors,
 	ms->leg_failure = 0;
 	atomic_set(&ms->suspend, 0);
 	atomic_set(&ms->default_mirror, DEFAULT_MIRROR);
+	burst = (DEFAULT_RESYNC_SPEED * (RESYNC_JIFFIES / HZ)) << 1;
+	burst = DIV_ROUND_UP(burst, region_size);
+	ratelimit_state_init(&ms->ms_rlimit, RESYNC_JIFFIES, burst);
+	INIT_DELAYED_WORK(&ms->resync_waker, do_resync_wake);
 
 	ms->read_record_pool = mempool_create_slab_pool(MIN_READ_RECORDS,
 						_dm_raid1_read_record_cache);
-- 
1.7.10.4

[PATCH 2/3 v3] dm raid1: add interface to set resync speed

[Guangliang Zhao]

Add ioctl to control resync speed, userspace tool
is dmsetup message, message format is:
	dmsetup message $device 0 "set-max-resync-speed $speed"
e.g.
	dmsetup message /dev/dm-2 0 "set-max-resync-speed 12345"

Signed-off-by: Guangliang Zhao <gzhao@suse.com>
---
 drivers/md/dm-raid1.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 43e428a..3cdad37 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -1361,6 +1361,49 @@ static void mirror_resume(struct dm_target *ti)
 }
 
 /*
+ * convert speed into members of ratelimit_state
+ */
+static void speed_to_rlimit(struct mirror_set *ms, struct ratelimit_state *rl,
+			    unsigned int speed)
+{
+	sector_t region_size = dm_rh_get_region_size(ms->rh);
+
+	rl->interval = RESYNC_JIFFIES;
+	rl->burst = (speed * (RESYNC_JIFFIES / HZ)) << 1;
+	rl->burst = DIV_ROUND_UP(rl->burst, region_size);
+}
+
+/*
+ * Message interface
+ *	set-max-resync-speed $speed(KB/s)
+ */
+static int mirror_message(struct dm_target *ti, unsigned argc, char **argv)
+{
+	struct mirror_set *ms = ti->private;
+	unsigned int speed;
+	int ret = 0;
+
+	if (!strcasecmp(argv[0], "set-max-resync-speed")) {
+		if (sscanf(argv[1], "%u", &speed) != 1) {
+			DMWARN("invalid speed parameter %s", argv[1]);
+			ret = -EINVAL;
+			goto error;
+		}
+
+		speed_to_rlimit(ms, &ms->ms_rlimit, speed);
+		goto out;
+	} else {
+		ret = -EINVAL;
+		goto error;
+	}
+
+error:
+	DMWARN("unrecognised message received.");
+out:
+	return ret;
+}
+
+/*
  * device_status_char
  * @m: mirror device/leg we want the status of
  *
@@ -1450,6 +1493,7 @@ static struct target_type mirror_target = {
 	.presuspend = mirror_presuspend,
 	.postsuspend = mirror_postsuspend,
 	.resume	 = mirror_resume,
+	.message = mirror_message,
 	.status	 = mirror_status,
 	.iterate_devices = mirror_iterate_devices,
 };
-- 
1.7.10.4

[PATCH 3/3 v3] dm raid1: add interface to get resync speed

[Guangliang Zhao]

Add ioctl to get resync speed, userspace tool
is dmsetup status:
	dmsetup status $device
e.g.
	dmsetup status /dev/dm-2

Signed-off-by: Guangliang Zhao <gzhao@suse.com>
---
 drivers/md/dm-raid1.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 3cdad37..62acfa3 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -1427,6 +1427,20 @@ static char device_status_char(struct mirror *m)
 		(test_bit(DM_RAID1_READ_ERROR, &(m->error_type))) ? 'R' : 'U';
 }
 
+/*
+ * get speed from ratelimit_state
+ */
+static unsigned int rlimit_to_speed(struct mirror_set *ms,
+				    struct ratelimit_state *rl)
+{
+	sector_t region_size = dm_rh_get_region_size(ms->rh);
+	unsigned int time, burst;
+
+	time = rl->interval / HZ;
+	burst = (rl->burst * region_size) >> 1;
+
+	return burst / time;
+}
 
 static int mirror_status(struct dm_target *ti, status_type_t type,
 			 char *result, unsigned int maxlen)
@@ -1451,6 +1465,8 @@ static int mirror_status(struct dm_target *ti, status_type_t type,
 
 		sz += log->type->status(log, type, result+sz, maxlen-sz);
 
+		DMEMIT(" %u", rlimit_to_speed(ms, &ms->ms_rlimit));
+
 		break;
 
 	case STATUSTYPE_TABLE:
-- 
1.7.10.4

Re: [dm-devel] [PATCH 0/3 v3] add resync speed control for dm-raid1

[Mikulas Patocka]

Hi

I did this already some times ago.
I'm sending my patches in the next mail.

Basically, my and Guangliang's patches have the following differences:

my patch: uses per-module throttle settings
Guangliang's patch: uses per-device settings
(my patch could be changed to use per-device throttle too, but without 
userspace support it isn't much useful because userspace lvm can 
reload the mirror and per-device settings would be lost)

my patch: uses fine grained throttling of the individual IOs in kcopyd - 
it measures active/inactive ratio and if the disk is active more than the 
specified percentage of time, sleep is inserted.
Guangliang's patch: throttles on segment granularity, it waits when 
starting new segment, but segment is copied unthrottled.

my patch: the user selects a percentage value (0 - 100) in 
"/sys/module/dm_mirror/parameters/raid1_resync_throttle", the device is 
kept active the specified percent of time
Guangliang's patch: limits the number of segments per a specified 
interval

My patch is noticeably bigger.

Mikulas


On Mon, 7 Jan 2013, Guangliang Zhao wrote:

> Hi,
> 
> These patches are used to add resync speed control for dm-raid1. The
> second and third patch provide support for user-space tool dmsetup.
> I have made some modifications by the comments. This is the third
> version.
> 
> Guangliang Zhao (3):
>   dm raid1: add resync speed control for dm-raid1
>   dm raid1: add interface to set resync speed
>   dm raid1: add interface to get resync speed
> 
>  drivers/md/dm-raid1.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> -- 
> 1.7.10.4
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

[PATCH 1/2] mirror throttling

[Mikulas Patocka]

dm-kcopyd: introduce per-module throttle structure

The structure contains the throttle parameter (it could be set in
/sys/module/*/parameters and auxulary variables for activity counting.

The throttle does nothing, it will be activated in the next patch.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-kcopyd.c    |    2 +-
 drivers/md/dm-raid1.c     |    5 ++++-
 drivers/md/dm-snap.c      |    5 ++++-
 drivers/md/dm-thin.c      |    5 ++++-
 include/linux/dm-kcopyd.h |   15 ++++++++++++++-
 5 files changed, 27 insertions(+), 5 deletions(-)

Index: linux-3.8-rc1-fast/drivers/md/dm-kcopyd.c
===================================================================
--- linux-3.8-rc1-fast.orig/drivers/md/dm-kcopyd.c	2013-01-02 23:00:49.000000000 +0100
+++ linux-3.8-rc1-fast/drivers/md/dm-kcopyd.c	2013-01-02 23:23:17.000000000 +0100
@@ -695,7 +695,7 @@ int kcopyd_cancel(struct kcopyd_job *job
 /*-----------------------------------------------------------------
  * Client setup
  *---------------------------------------------------------------*/
-struct dm_kcopyd_client *dm_kcopyd_client_create(void)
+struct dm_kcopyd_client *dm_kcopyd_client_create(struct dm_kcopyd_throttle *throttle)
 {
 	int r = -ENOMEM;
 	struct dm_kcopyd_client *kc;
Index: linux-3.8-rc1-fast/drivers/md/dm-raid1.c
===================================================================
--- linux-3.8-rc1-fast.orig/drivers/md/dm-raid1.c	2013-01-02 23:00:49.000000000 +0100
+++ linux-3.8-rc1-fast/drivers/md/dm-raid1.c	2013-01-02 23:23:17.000000000 +0100
@@ -82,6 +82,9 @@ struct mirror_set {
 	struct mirror mirror[0];
 };
 
+DECLARE_DM_KCOPYD_THROTTLE(raid1_resync_throttle,
+		"A percentage of time allocated for raid resynchronization");
+
 static void wakeup_mirrord(void *context)
 {
 	struct mirror_set *ms = context;
@@ -1111,7 +1114,7 @@ static int mirror_ctr(struct dm_target *
 		goto err_destroy_wq;
 	}
 
-	ms->kcopyd_client = dm_kcopyd_client_create();
+	ms->kcopyd_client = dm_kcopyd_client_create(&dm_kcopyd_throttle);
 	if (IS_ERR(ms->kcopyd_client)) {
 		r = PTR_ERR(ms->kcopyd_client);
 		goto err_destroy_wq;
Index: linux-3.8-rc1-fast/drivers/md/dm-snap.c
===================================================================
--- linux-3.8-rc1-fast.orig/drivers/md/dm-snap.c	2013-01-02 23:00:49.000000000 +0100
+++ linux-3.8-rc1-fast/drivers/md/dm-snap.c	2013-01-02 23:23:17.000000000 +0100
@@ -124,6 +124,9 @@ struct dm_snapshot {
 #define RUNNING_MERGE          0
 #define SHUTDOWN_MERGE         1
 
+DECLARE_DM_KCOPYD_THROTTLE(snapshot_copy_throttle,
+		"A percentage of time allocated for copy on write");
+
 struct dm_dev *dm_snap_origin(struct dm_snapshot *s)
 {
 	return s->origin;
@@ -1109,7 +1112,7 @@ static int snapshot_ctr(struct dm_target
 		goto bad_hash_tables;
 	}
 
-	s->kcopyd_client = dm_kcopyd_client_create();
+	s->kcopyd_client = dm_kcopyd_client_create(&dm_kcopyd_throttle);
 	if (IS_ERR(s->kcopyd_client)) {
 		r = PTR_ERR(s->kcopyd_client);
 		ti->error = "Could not create kcopyd client";
Index: linux-3.8-rc1-fast/include/linux/dm-kcopyd.h
===================================================================
--- linux-3.8-rc1-fast.orig/include/linux/dm-kcopyd.h	2013-01-02 22:59:41.000000000 +0100
+++ linux-3.8-rc1-fast/include/linux/dm-kcopyd.h	2013-01-02 23:23:17.000000000 +0100
@@ -21,11 +21,24 @@
 
 #define DM_KCOPYD_IGNORE_ERROR 1
 
+struct dm_kcopyd_throttle {
+	unsigned throttle;
+	unsigned long num_io_jobs;
+	unsigned io_period;
+	unsigned total_period;
+	unsigned last_jiffies;
+};
+
+#define DECLARE_DM_KCOPYD_THROTTLE(name, description)	\
+static struct dm_kcopyd_throttle dm_kcopyd_throttle = { 100, 0, 0, 0, 0 }; \
+module_param_named(name, dm_kcopyd_throttle.throttle, uint, 0644); \
+MODULE_PARM_DESC(name, description)
+
 /*
  * To use kcopyd you must first create a dm_kcopyd_client object.
  */
 struct dm_kcopyd_client;
-struct dm_kcopyd_client *dm_kcopyd_client_create(void);
+struct dm_kcopyd_client *dm_kcopyd_client_create(struct dm_kcopyd_throttle *throttle);
 void dm_kcopyd_client_destroy(struct dm_kcopyd_client *kc);
 
 /*
Index: linux-3.8-rc1-fast/drivers/md/dm-thin.c
===================================================================
--- linux-3.8-rc1-fast.orig/drivers/md/dm-thin.c	2013-01-02 23:00:49.000000000 +0100
+++ linux-3.8-rc1-fast/drivers/md/dm-thin.c	2013-01-02 23:23:17.000000000 +0100
@@ -26,6 +26,9 @@
 #define PRISON_CELLS 1024
 #define COMMIT_PERIOD HZ
 
+DECLARE_DM_KCOPYD_THROTTLE(snapshot_copy_throttle,
+		"A percentage of time allocated for copy on write");
+
 /*
  * The block size of the device holding pool data must be
  * between 64KB and 1GB.
@@ -1636,7 +1639,7 @@ static struct pool *pool_create(struct m
 		goto bad_prison;
 	}
 
-	pool->copier = dm_kcopyd_client_create();
+	pool->copier = dm_kcopyd_client_create(&dm_kcopyd_throttle);
 	if (IS_ERR(pool->copier)) {
 		r = PTR_ERR(pool->copier);
 		*error = "Error creating pool's kcopyd client";

[PATCH 2/2] mirror throttling

[Mikulas Patocka]

dm-kcopyd: use throttle

This patch allows the administrator to limit kcopyd rate.

We maintain a history of kcopyd usage in variables io_period and
total_period. The actual kcopyd activity is "(100 * io_period /
total_period)" percent of time. If we exceed user-defined percentage
threshold, we sleep.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-kcopyd.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

Index: linux-3.8-rc1-fast/drivers/md/dm-kcopyd.c
===================================================================
--- linux-3.8-rc1-fast.orig/drivers/md/dm-kcopyd.c	2013-01-02 23:23:17.000000000 +0100
+++ linux-3.8-rc1-fast/drivers/md/dm-kcopyd.c	2013-01-02 23:23:25.000000000 +0100
@@ -22,6 +22,7 @@
 #include <linux/vmalloc.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <linux/delay.h>
 #include <linux/device-mapper.h>
 #include <linux/dm-kcopyd.h>
 
@@ -51,6 +52,8 @@ struct dm_kcopyd_client {
 	struct workqueue_struct *kcopyd_wq;
 	struct work_struct kcopyd_work;
 
+	struct dm_kcopyd_throttle *throttle;
+
 /*
  * We maintain three lists of jobs:
  *
@@ -68,6 +71,108 @@ struct dm_kcopyd_client {
 
 static struct page_list zero_page_list;
 
+static DEFINE_SPINLOCK(throttle_spinlock);
+
+/*
+ * IO/IDLE accounting slowly decays after (1 << ACOUNT_INTERVAL_SHIFT) period.
+ * When total_period >= (1 << ACOUNT_INTERVAL_SHIFT) the counters are divided
+ * by 2.
+ */
+#define ACOUNT_INTERVAL_SHIFT		SHIFT_HZ
+
+/*
+ * Sleep this number of milliseconds.
+ *
+ * It is experimentally found value.
+ * Smaller values cause increased copy rate above the limit. The reason for
+ * this is unknown. A possible explanations could be jiffies rounding errors
+ * or read/write cache inside the disk.
+ */
+#define SLEEP_MSEC			100
+
+/*
+ * Maximum number of sleep events. There is a theoretical livelock if more
+ * kcopyd clients do work simultaneously, this limit allows us to get out of
+ * the livelock.
+ */
+#define MAX_SLEEPS			10
+
+static void io_job_start(struct dm_kcopyd_throttle *t)
+{
+	unsigned throttle, now, difference;
+	int slept, skew;
+
+	if (unlikely(!t))
+		return;
+
+	slept = 0;
+
+try_again:
+	spin_lock_irq(&throttle_spinlock);
+
+	throttle = ACCESS_ONCE(t->throttle);
+
+	if (likely(throttle >= 100))
+		goto skip_limit;
+
+	now = jiffies;
+	difference = now - t->last_jiffies;
+	t->last_jiffies = now;
+	if (t->num_io_jobs)
+		t->io_period += difference;
+	t->total_period += difference;
+
+	if (unlikely(t->total_period >= (1 << ACOUNT_INTERVAL_SHIFT))) {
+		int shift = fls(t->total_period >> ACOUNT_INTERVAL_SHIFT);
+		t->total_period >>= shift;
+		t->io_period >>= shift;
+	}
+
+	skew = t->io_period - throttle * t->total_period / 100;
+	/* skew = t->io_period * 100 / throttle - t->total_period; */
+	if (unlikely(skew > 0) && slept < MAX_SLEEPS) {
+		slept++;
+		spin_unlock_irq(&throttle_spinlock);
+		msleep(SLEEP_MSEC);
+		goto try_again;
+	}
+
+skip_limit:
+	t->num_io_jobs++;
+
+	spin_unlock_irq(&throttle_spinlock);
+}
+
+static void io_job_finish(struct dm_kcopyd_throttle *t)
+{
+	unsigned long flags;
+
+	if (unlikely(!t))
+		return;
+
+	spin_lock_irqsave(&throttle_spinlock, flags);
+
+	t->num_io_jobs--;
+
+	if (likely(ACCESS_ONCE(t->throttle) >= 100))
+		goto skip_limit;
+
+	if (!t->num_io_jobs) {
+		unsigned now, difference;
+
+		now = jiffies;
+		difference = now - t->last_jiffies;
+		t->last_jiffies = now;
+
+		t->io_period += difference;
+		t->total_period += difference;
+	}
+
+skip_limit:
+	spin_unlock_irqrestore(&throttle_spinlock, flags);
+}
+
+
 static void wake(struct dm_kcopyd_client *kc)
 {
 	queue_work(kc->kcopyd_wq, &kc->kcopyd_work);
@@ -348,6 +453,8 @@ static void complete_io(unsigned long er
 	struct kcopyd_job *job = (struct kcopyd_job *) context;
 	struct dm_kcopyd_client *kc = job->kc;
 
+	io_job_finish(kc->throttle);
+
 	if (error) {
 		if (job->rw & WRITE)
 			job->write_err |= error;
@@ -389,6 +496,8 @@ static int run_io_job(struct kcopyd_job 
 		.client = job->kc->io_client,
 	};
 
+	io_job_start(job->kc->throttle);
+
 	if (job->rw == READ)
 		r = dm_io(&io_req, 1, &job->source, NULL);
 	else
@@ -708,6 +817,7 @@ struct dm_kcopyd_client *dm_kcopyd_clien
 	INIT_LIST_HEAD(&kc->complete_jobs);
 	INIT_LIST_HEAD(&kc->io_jobs);
 	INIT_LIST_HEAD(&kc->pages_jobs);
+	kc->throttle = throttle;
 
 	kc->job_pool = mempool_create_slab_pool(MIN_JOBS, _job_cache);
 	if (!kc->job_pool)

Re: [dm-devel] [PATCH 0/3 v3] add resync speed control for dm-raid1

[Guangliang Zhao]

On Wed, Jan 09, 2013 at 12:43:21AM -0500, Mikulas Patocka wrote:
> Hi
Hi,

I think it is very good that your patches could be used for other
targets(snapshot, thin) after reviewing yours, but I find some issues
(maybe not, please correct me if I am wrong).

> 
> I did this already some times ago.
> I'm sending my patches in the next mail.
> 
> Basically, my and Guangliang's patches have the following differences:
> 
> my patch: uses per-module throttle settings
> Guangliang's patch: uses per-device settings
> (my patch could be changed to use per-device throttle too, but without 
> userspace support it isn't much useful because userspace lvm can 
> reload the mirror and per-device settings would be lost)

We couldn't force every devices in the system hold the same throttle,
IMHO, per-device settings couldn't be ignored. 
Setting the global value by the parameters of module is a good way, and
it could also be used to set the default value in my patches. In this way,
the global setting wouldn't be lost, and we could also adjust every device's
speed.

> 
> my patch: uses fine grained throttling of the individual IOs in kcopyd - 
> it measures active/inactive ratio and if the disk is active more than the 
> specified percentage of time, sleep is inserted.

I think this policy might not be able to represent the exact write speed, 
while other modules(such as md, drbd) monitor the real IO speed.

> Guangliang's patch: throttles on segment granularity, it waits when 
> starting new segment, but segment is copied unthrottled.
> 
> my patch: the user selects a percentage value (0 - 100) in 
> "/sys/module/dm_mirror/parameters/raid1_resync_throttle", the device is 
> kept active the specified percent of time
> Guangliang's patch: limits the number of segments per a specified 
> interval
> 
> My patch is noticeably bigger.
> 
> Mikulas
> 
> 
> On Mon, 7 Jan 2013, Guangliang Zhao wrote:
> 
> > Hi,
> > 
> > These patches are used to add resync speed control for dm-raid1. The
> > second and third patch provide support for user-space tool dmsetup.
> > I have made some modifications by the comments. This is the third
> > version.
> > 
> > Guangliang Zhao (3):
> >   dm raid1: add resync speed control for dm-raid1
> >   dm raid1: add interface to set resync speed
> >   dm raid1: add interface to get resync speed
> > 
> >  drivers/md/dm-raid1.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 89 insertions(+), 1 deletion(-)
> > 
> > -- 
> > 1.7.10.4
> > 
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
> > 

-- 
Best regards,
Guangliang

Re: [dm-devel] [PATCH 0/3 v3] add resync speed control for dm-raid1

[Mikulas Patocka]

On Wed, 16 Jan 2013, Guangliang Zhao wrote:

> On Wed, Jan 09, 2013 at 12:43:21AM -0500, Mikulas Patocka wrote:
> > Hi
> Hi,
> 
> I think it is very good that your patches could be used for other
> targets(snapshot, thin) after reviewing yours, but I find some issues
> (maybe not, please correct me if I am wrong).
> 
> > 
> > I did this already some times ago.
> > I'm sending my patches in the next mail.
> > 
> > Basically, my and Guangliang's patches have the following differences:
> > 
> > my patch: uses per-module throttle settings
> > Guangliang's patch: uses per-device settings
> > (my patch could be changed to use per-device throttle too, but without 
> > userspace support it isn't much useful because userspace lvm can 
> > reload the mirror and per-device settings would be lost)
> 
> We couldn't force every devices in the system hold the same throttle,
> IMHO, per-device settings couldn't be ignored. 
> Setting the global value by the parameters of module is a good way, and
> it could also be used to set the default value in my patches. In this way,
> the global setting wouldn't be lost, and we could also adjust every device's
> speed.

It could be good to have per-device throttle.

> > my patch: uses fine grained throttling of the individual IOs in kcopyd - 
> > it measures active/inactive ratio and if the disk is active more than the 
> > specified percentage of time, sleep is inserted.
> 
> I think this policy might not be able to represent the exact write speed, 
> while other modules(such as md, drbd) monitor the real IO speed.

But you don't want to limit raid resynchronization to a certain speed. A 
disk has varying speed, it is faster in the beginning and slower in the 
end.

So if you want to limit raid resynchronization so that other tasks have 
faster access to the disk, you need to limit percentage of time that is 
spent on resynchronization, not resynchronization speed.

Mikulas

Re: [dm-devel] [PATCH 0/3 v3] add resync speed control for dm-raid1

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

On Tue, 22 Jan 2013 20:44:41 -0500 (EST) Mikulas Patocka
<mpatocka@redhat.com> wrote:

> 
> 
> On Wed, 16 Jan 2013, Guangliang Zhao wrote:
> 
> > On Wed, Jan 09, 2013 at 12:43:21AM -0500, Mikulas Patocka wrote:
> > > Hi
> > Hi,
> > 
> > I think it is very good that your patches could be used for other
> > targets(snapshot, thin) after reviewing yours, but I find some issues
> > (maybe not, please correct me if I am wrong).
> > 
> > > 
> > > I did this already some times ago.
> > > I'm sending my patches in the next mail.
> > > 
> > > Basically, my and Guangliang's patches have the following differences:
> > > 
> > > my patch: uses per-module throttle settings
> > > Guangliang's patch: uses per-device settings
> > > (my patch could be changed to use per-device throttle too, but without 
> > > userspace support it isn't much useful because userspace lvm can 
> > > reload the mirror and per-device settings would be lost)
> > 
> > We couldn't force every devices in the system hold the same throttle,
> > IMHO, per-device settings couldn't be ignored. 
> > Setting the global value by the parameters of module is a good way, and
> > it could also be used to set the default value in my patches. In this way,
> > the global setting wouldn't be lost, and we could also adjust every device's
> > speed.
> 
> It could be good to have per-device throttle.
> 
> > > my patch: uses fine grained throttling of the individual IOs in kcopyd - 
> > > it measures active/inactive ratio and if the disk is active more than the 
> > > specified percentage of time, sleep is inserted.
> > 
> > I think this policy might not be able to represent the exact write speed, 
> > while other modules(such as md, drbd) monitor the real IO speed.
> 
> But you don't want to limit raid resynchronization to a certain speed. A 
> disk has varying speed, it is faster in the beginning and slower in the 
> end.
> 
> So if you want to limit raid resynchronization so that other tasks have 
> faster access to the disk, you need to limit percentage of time that is 
> spent on resynchronization, not resynchronization speed.

Sounds good ..... not that easy though.

But if the disk is otherwise idle, I want 100% of the time to be spend on
synchronisation.  If it isn't otherwise idle, I want a much more modest
faction to be used.

Getting this "right" is very hard.  You want to resync aggressively if there
is no other traffic, but to back off quickly to some small background rate if
there is any other traffic.  That is what md tries to do.

dm-raid1 has an extra complication.  It is used in clusters (clvm) where
multiple separate hosts might be accessing the device.  So the host which is
driving the resync cannot know what other IO there might be.
In that case the only thing that seems to be practical is an maximum sync
speed that can be set by the admin.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [dm-devel] [PATCH 0/3 v3] add resync speed control for dm-raid1

[Mikulas Patocka]

On Wed, 23 Jan 2013, NeilBrown wrote:

> On Tue, 22 Jan 2013 20:44:41 -0500 (EST) Mikulas Patocka
> <mpatocka@redhat.com> wrote:
> 
> > 
> > 
> > On Wed, 16 Jan 2013, Guangliang Zhao wrote:
> > 
> > > On Wed, Jan 09, 2013 at 12:43:21AM -0500, Mikulas Patocka wrote:
> > > > Hi
> > > Hi,
> > > 
> > > I think it is very good that your patches could be used for other
> > > targets(snapshot, thin) after reviewing yours, but I find some issues
> > > (maybe not, please correct me if I am wrong).
> > > 
> > > > 
> > > > I did this already some times ago.
> > > > I'm sending my patches in the next mail.
> > > > 
> > > > Basically, my and Guangliang's patches have the following differences:
> > > > 
> > > > my patch: uses per-module throttle settings
> > > > Guangliang's patch: uses per-device settings
> > > > (my patch could be changed to use per-device throttle too, but without 
> > > > userspace support it isn't much useful because userspace lvm can 
> > > > reload the mirror and per-device settings would be lost)
> > > 
> > > We couldn't force every devices in the system hold the same throttle,
> > > IMHO, per-device settings couldn't be ignored. 
> > > Setting the global value by the parameters of module is a good way, and
> > > it could also be used to set the default value in my patches. In this way,
> > > the global setting wouldn't be lost, and we could also adjust every device's
> > > speed.
> > 
> > It could be good to have per-device throttle.
> > 
> > > > my patch: uses fine grained throttling of the individual IOs in kcopyd - 
> > > > it measures active/inactive ratio and if the disk is active more than the 
> > > > specified percentage of time, sleep is inserted.
> > > 
> > > I think this policy might not be able to represent the exact write speed, 
> > > while other modules(such as md, drbd) monitor the real IO speed.
> > 
> > But you don't want to limit raid resynchronization to a certain speed. A 
> > disk has varying speed, it is faster in the beginning and slower in the 
> > end.
> > 
> > So if you want to limit raid resynchronization so that other tasks have 
> > faster access to the disk, you need to limit percentage of time that is 
> > spent on resynchronization, not resynchronization speed.
> 
> Sounds good ..... not that easy though.
> 
> But if the disk is otherwise idle, I want 100% of the time to be spend on
> synchronisation.  If it isn't otherwise idle, I want a much more modest
> faction to be used.

So, hack the i/o scheduler - the scheduler can make such decisions, it is 
impossible to do this decision in the dm-mirror layer, because the 
dm-mirror layer has no knowledge of how much is the disk loaded.

It is clearly better solution to solve it in the scheduler, this throttle 
is a simple solution for the case when the i/o scheduler isn't right.

> Getting this "right" is very hard.  You want to resync aggressively if there
> is no other traffic, but to back off quickly to some small background rate if
> there is any other traffic.  That is what md tries to do.
> 
> dm-raid1 has an extra complication.  It is used in clusters (clvm) where
> multiple separate hosts might be accessing the device.  So the host which is
> driving the resync cannot know what other IO there might be.

The same problem arises on a single computer - there may be multiple 
logical volumes or multiple partitions on the same disk.

> In that case the only thing that seems to be practical is an maximum sync
> speed that can be set by the admin.

Maximum speed sync doesn't work well because the disk has a different 
speed in the beginning and in the end. For example, if you set maximum 
speed as 50% of the resync speed when you start resynchronizing, the 
throttle stops working at the end of the disk, because the disk itself is 
50% slower at the end.

A better solution is to limit the time spent doing i/o. If you set it to 
50%, it is 50% loaded and 50% idle for the whole duration of the 
resynchronization, regardless of different transfer speed in each disk 
region.

> NeilBrown

Mikulas

Re: [PATCH 2/2] mirror throttling

[Alasdair G Kergon]

On Wed, Jan 09, 2013 at 12:44:38AM -0500, Mikulas Patocka wrote:
> We maintain a history of kcopyd usage in variables io_period and
> total_period. The actual kcopyd activity is "(100 * io_period /
> total_period)" percent of time. If we exceed user-defined percentage
> threshold, we sleep.
 
Well, I'm going to take this pair of patches for now.

Some people do need this throttling today and this seems to me to be a
decent and simple way to give them a lever to deal with the problem.

I'm not proposing we expose this through LVM or other userspace tools at
this stage: people who need it should tune it directly through sysfs.

If need be, we can revisit this in future either by refining the
algorithm or making it adjustable per-device rather than per-module.
(Or by re-vamping kcopyd itself...)

The current version is:
  http://people.redhat.com/agk/patches/linux/editing/dm-kcopyd-introduce-configurable-throttling.patch

Alasdair