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
| ||
|
Date: Thu, 18 Mar 2010 09:05:00 +0800 From: Shaohua Li <shaohua.li@...el.com> To: Corrado Zoccolo <czoccolo@...il.com> Cc: Jens Axboe <jens.axboe@...cle.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "vgoyal@...hat.com" <vgoyal@...hat.com>, "jmoyer@...hat.com" <jmoyer@...hat.com>, "guijianfeng@...fujitsu.com" <guijianfeng@...fujitsu.com>, "Shi, Alex" <alex.shi@...el.com> Subject: Re: [RFC]cfq-iosched: fix a kbuild regression On Thu, Mar 18, 2010 at 02:24:10AM +0800, Corrado Zoccolo wrote: > On Wed, Mar 17, 2010 at 2:30 PM, Jens Axboe <jens.axboe@...cle.com> wrote: > > On Tue, Mar 16 2010, Shaohua Li wrote: > >> Alex Shi reported a kbuild regression which is about 10% performance lost. > >> He bisected to this commit: 3dde36ddea3e07dd025c4c1ba47edec91606fec0. > >> The reason is cfqq_close() can't find close cooperator. If we store the seek > >> distance to the value before the commit like below, the regression fully goes > >> away. If this is too invasive, just changing the cfq_rq_close() for the > >> !for_preempt is ok too. > > > > Corrado, any objections to widening the seek threshold? > > The previous seek threshold value was meant to be compared with the > average seek distance, so it was large to account for variations. > Since we handle the variations differently, we should have a smaller > value now (the 100 * 8 was the result of a benchmark run on several > disks). Our test doesn't find any issue with the seek threshold so far, so it proberly is ok. > I agreee, though, that for merging queues, it is now too small, so we > should have two thresholds, the current one used to determine if a > request causes a seek, and an other to be used inside cfq_close (with > the older value, regardless of for_preempt). That makes sense. Updated patch. Alex Shi reported a kbuild regression which is about 10% performance lost. He bisected to this commit: 3dde36ddea3e07dd025c4c1ba47edec91606fec0. The reason is cfqq_close() can't find close cooperator. Restoring cfq_rq_close()'s threshold to original value makes the regression go away. Since for_preempt parameter isn't used anymore, this patch deletes it. Reported-by: Alex Shi <alex.shi@...el.com> Signed-off-by: Shaohua Li <shaohua.li@...el.com> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index dee9d93..8d5a2f2 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -47,6 +47,7 @@ static const int cfq_hist_divisor = 4; #define CFQ_SERVICE_SHIFT 12 #define CFQQ_SEEK_THR (sector_t)(8 * 100) +#define CFQQ_CLOSE_THR (sector_t)(8 * 1024) #define CFQQ_SECT_THR_NONROT (sector_t)(2 * 32) #define CFQQ_SEEKY(cfqq) (hweight32(cfqq->seek_history) > 32/8) @@ -1660,9 +1661,9 @@ static inline sector_t cfq_dist_from_last(struct cfq_data *cfqd, } static inline int cfq_rq_close(struct cfq_data *cfqd, struct cfq_queue *cfqq, - struct request *rq, bool for_preempt) + struct request *rq) { - return cfq_dist_from_last(cfqd, rq) <= CFQQ_SEEK_THR; + return cfq_dist_from_last(cfqd, rq) <= CFQQ_CLOSE_THR; } static struct cfq_queue *cfqq_close(struct cfq_data *cfqd, @@ -1689,7 +1690,7 @@ static struct cfq_queue *cfqq_close(struct cfq_data *cfqd, * will contain the closest sector. */ __cfqq = rb_entry(parent, struct cfq_queue, p_node); - if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq, false)) + if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq)) return __cfqq; if (blk_rq_pos(__cfqq->next_rq) < sector) @@ -1700,7 +1701,7 @@ static struct cfq_queue *cfqq_close(struct cfq_data *cfqd, return NULL; __cfqq = rb_entry(node, struct cfq_queue, p_node); - if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq, false)) + if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq)) return __cfqq; return NULL; @@ -3103,7 +3104,7 @@ cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq, * if this request is as-good as one we would expect from the * current cfqq, let it preempt */ - if (cfq_rq_close(cfqd, cfqq, rq, true)) + if (cfq_rq_close(cfqd, cfqq, rq)) return true; return false; -- 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/
Powered by blists - more mailing lists