>From heinzm@xxxxxxxxxx Wed Aug 11 17:19:52 2010 Subject: Re: [PATCH} dm-throttle: new device mapper target to throttle reads and writes From: Heinz Mauelshagen <heinzm@xxxxxxxxxx> Reply-To: heinzm@xxxxxxxxxx To: Vivek Goyal <vgoyal@xxxxxxxxxxx> Cc: dm-devel@xxxxxxxxxxx In-Reply-To: <20100810184138.GC9028@xxxxxxxxxx> References: <1281447742.4964.46.camel@o> <20100810184138.GC9028@xxxxxxxxxx> Content-Type: text/plain; charset="UTF-8" Organization: Red Hat Inc. Message-ID: <1281539990.4964.328.camel@o> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 (2.28.3-1.fc12) Date: Wed, 11 Aug 2010 17:19:52 +0200 X-Evolution-Format: text/plain X-Evolution-Account: 1270744278.6871.1@o X-Evolution-Transport: smtp://heinzm;auth=PLAIN@xxxxxxxxxxxxxxxxxxxx/;use_ssl=never X-Evolution-Fcc: mbox:/home/mauelsha/.evolution/mail/local#Sent Content-Transfer-Encoding: 8bit On Tue, 2010-08-10 at 14:41 -0400, Vivek Goyal wrote: > On Tue, Aug 10, 2010 at 03:42:22PM +0200, Heinz Mauelshagen wrote: > > [..] > > +/* Decide about throttling (ie. deferring bios). */ > > +static int throttle(struct throttle_c *tc, struct bio *bio) > > +{ > > + int rw = (bio_data_dir(bio) == WRITE); > > + unsigned bps; /* Bytes per second. */ > > + > > + smp_rmb(); > > + bps = tc->params.bs[rw]; > > + if (bps) { > > + unsigned size; > > + struct account *ac = &tc->account; > > + struct ac_rw *ac_rw = ac->rw + rw; > > + > > + if (time_after(jiffies, ac_rw->end_jiffies)) > > + /* Measure time exceeded. */ > > + account_reset(rw, tc); > > + else if (test_bit(rw, &ac->flags)) > > + /* In case we're throttled already. */ > > + return 1; > > + > > + /* Account I/O size. */ > > + size = ac_rw->size + bio->bi_size; > > + if (size > bps) { > > + /* Hit kilobytes per second threshold. */ > > + set_bit(rw, &ac->flags); > > + return 1; > > If bio->bi_size is greate than bps, will I always keep on throttling > and hang? bps needs to be set larger than the bio maximum size expected with the current implementatio, right. The algorithm needs changing to cope with bi_size larger than bps (see below). > > [..] > > +/* Map a throttle io. */ > > +static int throttle_map(struct dm_target *ti, struct bio *bio, > > + union map_info *map_context) > > +{ > > + int r, rw = (bio_data_dir(bio) == WRITE); > > + struct throttle_c *tc = ti->private; > > + struct ac_rw *ac_rw = tc->account.rw + rw; > > + > > + mutex_lock(&ac_rw->mutex); > > + do { > > + r = throttle(tc, bio); > > + if (r) { > > + long end = ac_rw->end_jiffies, j = jiffies; > > + > > + /* Wait till next second when KB/s reached. */ > > + if (j < end) > > + schedule_timeout_uninterruptible(end - j); > > + } > > So a thread is blocked if it crossed the IO rate. There is no such mechanism > to take the bio, statsh away somewhere and dispatch it to disk later. The > way request queue descriptors work. Right, the aim for this testing target was to keep it as simple as possible to solve the purpose of simulating low bandwidth transports or varying device throughput properties. Cheap approaches to tackle this issue include to set ti->split_io based on bps < BIO_MAX_SIZE (units ignored) in the ctr/message interface, to prohibit bps smaller than BIO_MAX_SIZE altogether or to change the throttle() algorithm to allow for > 1s measurement periods based on bi_size maximum vs. bps ratios. The 1st one obviously causing more bio splits, the 2nd one prohibiting small bandwidth simulation and the last one causing io peeks, which is actually not what I wanted. > Processes are blocked only if queue > is congested otherwise one allows processes to submit requests and go > back and do other work. > > I am assuming that this will be bad for AIO. Yes, bio stashing/dispatching mandatory to make AIO work Regards, Heinz > > Thanks > Vivek -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
