linux-kernel - Re: Re: blk-throttle.c : When limit is changed, must start a new slice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <201103092340154215777@foxmail.com>
Date:	Wed, 9 Mar 2011 23:40:16 +0800
From:	"lulina_nuaa" <lulina_nuaa@...mail.com>
To:	"Vivek Goyal" <vgoyal@...hat.com>
Cc:	"linux kernel mailing list" <linux-kernel@...r.kernel.org>
Subject: Re: Re: blk-throttle.c : When limit is changed, must start a new slice

>On 2011-03-09 04:54:43, Vivek Goyal wrote:
>
>On Tue, Mar 08, 2011 at 11:03:59PM +0800, lina wrote:

>[..]
>> >>  Unfortunately, the following patch still has 5~10 seconds latency. I have no
>> >>  idea to resolve this problem, it seens hard to find a more suitable func to
>> >>  call throtl_start_new_slice().
>> >
>> >So are you saying that following patch did not solve the latnecy issue?
>> >Resetting slice upon limit change did not work for you?
>> >
>>   
>>  Yes, the following patch did not solve the latency issue. There is still 5~10
>>  seconds latency when I change the limit from a very high value to low. From
>>  blktrace, I find that the throtl_process_limit_change() is called after work 
>>  queue delay.
>>   
>>  Thanks
>>  Lina
>
>Ok,
>
>Can you try the attached patch. I think what was happening that after
>changing limits, work was not being scheduled as there were no queued
>bios hence no slice reset was taking place immediately.
>
>[..]
>
>Thanks
>Vivek
>

I have remove the HTML code, I'm sorry for the mail format!

Thank you very much for the following patch! I think it can solve the problem.
I'll test it as soon as possible, and will inform you once get the result!

Thanks
Lina

>---
> block/blk-throttle.c |   24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
>Index: linux-2.6/block/blk-throttle.c
>===================================================================
>--- linux-2.6.orig/block/blk-throttle.c	2011-03-04 13:59:45.000000000 -0500
>+++ linux-2.6/block/blk-throttle.c	2011-03-08 15:41:19.384654732 -0500
>@@ -757,6 +757,14 @@ static void throtl_process_limit_change(
> 				" riops=%u wiops=%u", tg->bps[READ],
> 				tg->bps[WRITE], tg->iops[READ],
> 				tg->iops[WRITE]);
>+			/*
>+			 * Restart the slices for both READ and WRITES. It
>+			 * might happen that a group's limit are dropped
>+			 * suddenly and we don't want to account recently
>+			 * dispatched IO with new low rate
>+			 */
>+			throtl_start_new_slice(td, tg, 0);
>+			throtl_start_new_slice(td, tg, 1);
> 			tg_update_disptime(td, tg);
> 			tg->limits_changed = false;
> 		}
>@@ -825,7 +833,8 @@ throtl_schedule_delayed_work(struct thro
> 
> 	struct delayed_work *dwork = &td->throtl_work;
> 
>-	if (total_nr_queued(td) > 0) {
>+	/* schedule work if limits changed even if no bio is queued */
>+	if (total_nr_queued(td) > 0 || atomic_read(&td->limits_changed)) {
> 		/*
> 		 * We might have a work scheduled to be executed in future.
> 		 * Cancel that and schedule a new one.
>@@ -1023,6 +1032,19 @@ int blk_throtl_bio(struct request_queue 
> 	/* Bio is with-in rate limit of group */
> 	if (tg_may_dispatch(td, tg, bio, NULL)) {
> 		throtl_charge_bio(tg, bio);
>+
>+		/*
>+		 * We need to trim slice even when bios are not being queued
>+		 * otherwise it might happen that a bio is not queued for
>+		 * a long time and slice keeps on extending and trim is not
>+		 * called for a long time. Now if limits are reduced suddenly
>+		 * we take into account all the IO dispatched so far at new
>+		 * low rate and * newly queued IO gets a really long dispatch
>+		 * time.
>+		 *
>+		 * So keep on trimming slice even if bio is not queued.
>+		 */
>+		throtl_trim_slice(td, tg, rw);
> 		goto out;
> 	}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/