Hi

Here I'm sending three patches that limit kcopyd speed. There is global
limit for all kcopyds running in a system. The user can set percentage of
kcopyd speed in /sys/module/dm_mod/parameters/dm_kcopyd_throttle

Mikulas

---

dm-kcopyd: monitor io activity

There are two activity counters, "total_period" and "io_period".
total_period counts all time ticks, io_period counts timer ticks when
some I/O is active.

Thus, (100 * io_period / total_period) represents the percentage of
time when kcopyd is active.

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

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

Index: linux-2.6.39-fast/drivers/md/dm-kcopyd.c
================================================== =================
--- linux-2.6.39-fast.orig/drivers/md/dm-kcopyd.c 2011-05-30 18:04:08.000000000 +0200
+++ linux-2.6.39-fast/drivers/md/dm-kcopyd.c 2011-05-30 18:07:36.000000000 +0200
@@ -66,6 +66,69 @@ struct dm_kcopyd_client {
struct list_head pages_jobs;
};

+static DEFINE_SPINLOCK(activity_spinlock);
+static unsigned long num_io_jobs = 0;
+
+/*
+ * kcopyd is active (100 * io_period / total_period) percent of time.
+ */
+static unsigned io_period = 0;
+static unsigned total_period = 0;
+static unsigned last_jiffies = 0;
+
+/*
+ * 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
+
+static void io_job_start(void)
+{
+ unsigned now, difference;
+
+ spin_lock_irq(&activity_spinlock);
+
+ now = jiffies;
+ difference = now - last_jiffies;
+ last_jiffies = now;
+ if (num_io_jobs)
+ io_period += difference;
+ total_period += difference;
+
+ if (unlikely(total_period >= (1 << ACOUNT_INTERVAL_SHIFT))) {
+ int shift = fls(total_period >> ACOUNT_INTERVAL_SHIFT);
+ total_period >>= shift;
+ io_period >>= shift;
+ }
+
+ num_io_jobs++;
+
+ spin_unlock_irq(&activity_spinlock);
+}
+
+static void io_job_finish(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&activity_spinlock, flags);
+
+ num_io_jobs--;
+
+ if (!num_io_jobs) {
+ unsigned now, difference;
+
+ now = jiffies;
+ difference = now - last_jiffies;
+ last_jiffies = now;
+
+ io_period += difference;
+ total_period += difference;
+ }
+
+ spin_unlock_irqrestore(&activity_spinlock, flags);
+}
+
static void wake(struct dm_kcopyd_client *kc)
{
queue_work(kc->kcopyd_wq, &kc->kcopyd_work);
@@ -324,6 +387,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();
+
if (error) {
if (job->rw == WRITE)
job->write_err |= error;
@@ -365,6 +430,8 @@ static int run_io_job(struct kcopyd_job
.client = job->kc->io_client,
};

+ io_job_start();
+
if (job->rw == READ)
r = dm_io(&io_req, 1, &job->source, NULL);
else

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Mon, May 30, 2011 at 12:40:39PM -0400, Mikulas Patocka wrote:
> Here I'm sending three patches that limit kcopyd speed. There is global
> limit for all kcopyds running in a system. The user can set percentage of
> kcopyd speed in /sys/module/dm_mod/parameters/dm_kcopyd_throttle

Could you give some explanation of how to choose an appropriate value
for this please? Should it be 100% unless there is an issue? How
does the sys admin spot such issues?

Different kcopyd clients have different performance requirements, for
instance the initial sync of a new mirror leg may have less priority
than a copy-on-write exception for a snapshot. This isn't something
we've addressed so far, but if you're starting to slow kcopyd down
(i.e. so the mirror sync doesn't saturate the system), then I think
you'll get some complaints from the snapshot users.

- joe

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Tue, 31 May 2011, Joe Thornber wrote:

> On Tue, May 31, 2011 at 10:12:24AM +0100, Alasdair G Kergon wrote:
> > On Tue, May 31, 2011 at 10:09:01AM +0100, Joe Thornber wrote:
> > > Different kcopyd clients have different performance requirements, for
> > > instance the initial sync of a new mirror leg may have less priority
> > > than a copy-on-write exception for a snapshot. This isn't something
> > > we've addressed so far, but if you're starting to slow kcopyd down
> > > (i.e. so the mirror sync doesn't saturate the system), then I think
> > > you'll get some complaints from the snapshot users.

I think it's a good comment. We could make different throttles for
different clases of clients --- i.e. have one throttle for mirrors, one
for snapshots, one for multisnap, etc.

The variables "dm_kcopyd_throttle", "activity_spinlock", "num_io_jobs",
"io_period", "total_period", "last_jiffies" could be placed into a
structure and the structure could be moved to every module that uses
kcopyd. Thus, the modules could be throttled independently.

> > Is it plausible to differentiate between the classes of clients,
> > and prioritise snapshot i/o above mirror or have different throttling
> > parameters?
>
> Well I think it's plausible to have a priority field as part of the
> request structure. This could be either a direct priority, or a
> 'class' mapping onto a throttling parameter as you suggest. As long
> as we're careful to avoid starvation it shouldn't be too hard to
> implement.
>
> In general I'd rather avoid introducing paramaters for the sys admin.

If you don't make a parameter, the whole idea of throttling is useless.
The throttling should happen only at admin's request, never do it
automatically. If we slow down I/O deliberately, people will surely come
up with scenarios where it hurts performance.

> It's more work for them, and in my experience they rarely get set
> properly. So is there a way to infer priorities for different
> clients? Perhaps a function of number and size of recently submitted
> kcopyd jobs?

This is duplicating work of i/o scheduler. Such decisions should be made
in the i/o scheduler and not in kcopyd.

> If one client has submitted a request to copy 64k of
> data and the other 1G does it always make sense to do the small job in
> a more timely manner?
>
> - Joe

Mikulas

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel