* Re: [PATCH Linux 2.6.12-rc4-mm2 01/03] cfq: cfq ELEVATOR_INSERT_BACK fix
2005-05-24 16:35 [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Tejun Heo
@ 2005-05-24 16:35 ` Tejun Heo
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 02/03] cfq: cfq_io_context leak fix Tejun Heo
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2005-05-24 16:35 UTC (permalink / raw)
To: axboe; +Cc: linux-kernel
01_cfq_INSERT_BACK_fix.patch
When inserting INSERT_BACK request, cfq_insert_request() calls
cfq_dispatch_requests() repetitively until it returns zero
indicating no request is dispatched. This used to flush all
the requests in the queue to the dispatch queue but, with idle
slice implemented, the current active queue may decide to wait
for new request using slice_timer. When this happens, 0 is
returned from cfq_dispatch_requests() even when other cfqq's
have pending requests. This breaks INSRET_BACK semantics.
This patch adds @force argument which, when set to non-zero,
disables idle_slice, and uses the argument when flushing
cfqq's for INSERT_BACK. While at it, use INT_MAX instead of
cfq_quantum when flushing cfqq's, as we're gonna dump all the
requests and using cfq_quantum does nothing but adding
unnecessary iterations.
Signed-off-by: Tejun Heo <htejun@gmail.com>
cfq-iosched.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
Index: blk-fixes/drivers/block/cfq-iosched.c
===================================================================
--- blk-fixes.orig/drivers/block/cfq-iosched.c 2005-05-25 01:35:16.000000000 +0900
+++ blk-fixes/drivers/block/cfq-iosched.c 2005-05-25 01:35:16.000000000 +0900
@@ -991,7 +991,7 @@ cfq_prio_to_maxrq(struct cfq_data *cfqd,
/*
* get next queue for service
*/
-static struct cfq_queue *cfq_select_queue(struct cfq_data *cfqd)
+static struct cfq_queue *cfq_select_queue(struct cfq_data *cfqd, int force)
{
unsigned long now = jiffies;
struct cfq_queue *cfqq;
@@ -1012,7 +1012,8 @@ static struct cfq_queue *cfq_select_queu
*/
if (!RB_EMPTY(&cfqq->sort_list))
goto keep_queue;
- else if (cfq_cfqq_sync(cfqq) && time_before(now, cfqq->slice_end)) {
+ else if (!force && cfq_cfqq_sync(cfqq) &&
+ time_before(now, cfqq->slice_end)) {
if (cfq_arm_slice_timer(cfqd, cfqq))
return NULL;
}
@@ -1078,7 +1079,8 @@ __cfq_dispatch_requests(struct cfq_data
return dispatched;
}
-static int cfq_dispatch_requests(request_queue_t *q, int max_dispatch)
+static int
+cfq_dispatch_requests(request_queue_t *q, int max_dispatch, int force)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
struct cfq_queue *cfqq;
@@ -1086,7 +1088,7 @@ static int cfq_dispatch_requests(request
if (!cfqd->busy_queues)
return 0;
- cfqq = cfq_select_queue(cfqd);
+ cfqq = cfq_select_queue(cfqd, force);
if (cfqq) {
cfqq->wait_request = 0;
cfqq->must_dispatch = 0;
@@ -1172,7 +1174,7 @@ dispatch:
return rq;
}
- if (cfq_dispatch_requests(q, cfqd->cfq_quantum))
+ if (cfq_dispatch_requests(q, cfqd->cfq_quantum, 0))
goto dispatch;
return NULL;
@@ -1707,7 +1709,7 @@ cfq_insert_request(request_queue_t *q, s
switch (where) {
case ELEVATOR_INSERT_BACK:
- while (cfq_dispatch_requests(q, cfqd->cfq_quantum))
+ while (cfq_dispatch_requests(q, INT_MAX, 1))
;
list_add_tail(&rq->queuelist, &q->queue_head);
break;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH Linux 2.6.12-rc4-mm2 02/03] cfq: cfq_io_context leak fix
2005-05-24 16:35 [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Tejun Heo
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 01/03] cfq: cfq ELEVATOR_INSERT_BACK fix Tejun Heo
@ 2005-05-24 16:35 ` Tejun Heo
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 03/03] cfq: remove serveral unused fields from cfq data structures Tejun Heo
2005-05-25 7:34 ` [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2005-05-24 16:35 UTC (permalink / raw)
To: axboe; +Cc: linux-kernel
02_cfq_ioc_leak_fix.patch
When a process has more than one cic's associated with it,
only the first one was kmem_cache_free'd in the original code.
This patch frees all cic's in cfq_free_io_context().
While at it, remove unnecessary refcounting from cic's to ioc.
This reference is created when each cic is created and removed
altogether when the ioc is exited, and, thus, serves no
purpose.
Signed-off-by: Tejun Heo <htejun@gmail.com>
cfq-iosched.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
Index: blk-fixes/drivers/block/cfq-iosched.c
===================================================================
--- blk-fixes.orig/drivers/block/cfq-iosched.c 2005-05-25 01:35:16.000000000 +0900
+++ blk-fixes/drivers/block/cfq-iosched.c 2005-05-25 01:35:16.000000000 +0900
@@ -1238,6 +1238,14 @@ cfq_find_cfq_hash(struct cfq_data *cfqd,
static void cfq_free_io_context(struct cfq_io_context *cic)
{
+ struct cfq_io_context *__cic;
+ struct list_head *entry, *next;
+
+ list_for_each_safe(entry, next, &cic->list) {
+ __cic = list_entry(entry, struct cfq_io_context, list);
+ kmem_cache_free(cfq_ioc_pool, __cic);
+ }
+
kmem_cache_free(cfq_ioc_pool, cic);
}
@@ -1260,7 +1268,6 @@ static void cfq_exit_single_io_context(s
cfq_put_queue(cic->cfqq);
cic->cfqq = NULL;
- put_io_context(cic->ioc);
spin_unlock(q->queue_lock);
}
@@ -1271,7 +1278,7 @@ static void cfq_exit_single_io_context(s
static void cfq_exit_io_context(struct cfq_io_context *cic)
{
struct cfq_io_context *__cic;
- struct list_head *entry, *nxt;
+ struct list_head *entry;
unsigned long flags;
local_irq_save(flags);
@@ -1279,10 +1286,8 @@ static void cfq_exit_io_context(struct c
/*
* put the reference this task is holding to the various queues
*/
- list_for_each_safe(entry, nxt, &cic->list) {
+ list_for_each(entry, &cic->list) {
__cic = list_entry(entry, struct cfq_io_context, list);
-
- list_del(&__cic->list);
cfq_exit_single_io_context(__cic);
}
@@ -1471,8 +1476,6 @@ cfq_get_io_context(struct cfq_data *cfqd
ioc->cic = cic;
ioc->set_ioprio = cfq_ioc_set_ioprio;
cic->ioc = ioc;
- atomic_inc(&ioc->refcount);
-
cic->key = cfqd;
atomic_inc(&cfqd->ref);
} else {
@@ -1509,8 +1512,6 @@ cfq_get_io_context(struct cfq_data *cfqd
goto err;
__cic->ioc = ioc;
- atomic_inc(&ioc->refcount);
-
__cic->key = cfqd;
atomic_inc(&cfqd->ref);
list_add(&__cic->list, &cic->list);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH Linux 2.6.12-rc4-mm2 03/03] cfq: remove serveral unused fields from cfq data structures
2005-05-24 16:35 [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Tejun Heo
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 01/03] cfq: cfq ELEVATOR_INSERT_BACK fix Tejun Heo
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 02/03] cfq: cfq_io_context leak fix Tejun Heo
@ 2005-05-24 16:35 ` Tejun Heo
2005-05-25 7:34 ` [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2005-05-24 16:35 UTC (permalink / raw)
To: axboe; +Cc: linux-kernel
03_cfq_remove_unused_fields.patch
cfq_data->idle_start, cfq_data->end_prio and cfq_rq->end_pos
are not used in meaningful way. Remove'em.
Signed-off-by: Tejun Heo <htejun@gmail.com>
cfq-iosched.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
Index: blk-fixes/drivers/block/cfq-iosched.c
===================================================================
--- blk-fixes.orig/drivers/block/cfq-iosched.c 2005-05-25 01:35:16.000000000 +0900
+++ blk-fixes/drivers/block/cfq-iosched.c 2005-05-25 01:35:17.000000000 +0900
@@ -143,11 +143,10 @@ struct cfq_data {
*/
struct timer_list idle_slice_timer;
struct work_struct unplug_work;
- unsigned long idle_start;
struct cfq_queue *active_queue;
struct cfq_io_context *active_cic;
- int cur_prio, cur_end_prio, end_prio;
+ int cur_prio, cur_end_prio;
unsigned int dispatch_slice;
struct timer_list idle_class_timer;
@@ -234,8 +233,6 @@ struct cfq_rq {
struct cfq_queue *cfq_queue;
struct cfq_io_context *io_context;
- sector_t end_pos;
-
unsigned in_flight : 1;
unsigned accounted : 1;
unsigned is_sync : 1;
@@ -770,12 +767,9 @@ static int cfq_get_next_prio_level(struc
cfqd->cur_end_prio = cfqd->cur_prio;
cfqd->cur_prio = 0;
}
- if (cfqd->cur_end_prio > cfqd->end_prio)
- cfqd->end_prio = cfqd->cur_end_prio;
- if (cfqd->end_prio == CFQ_PRIO_LISTS) {
+ if (cfqd->cur_end_prio == CFQ_PRIO_LISTS) {
cfqd->cur_prio = 0;
cfqd->cur_end_prio = 0;
- cfqd->end_prio = 0;
}
return prio;
@@ -876,7 +870,6 @@ static int cfq_arm_slice_timer(struct cf
if (!timer_pending(&cfqd->idle_slice_timer)) {
unsigned long slice_left = cfqq->slice_end - 1;
- cfqd->idle_start = jiffies;
cfqd->idle_slice_timer.expires = min(jiffies + cfqd->cfq_slice_idle, slice_left);
add_timer(&cfqd->idle_slice_timer);
}
@@ -926,7 +919,6 @@ static void cfq_dispatch_sort(request_qu
cfq_del_crq_rb(crq);
cfq_remove_merge_hints(q, crq);
- crq->end_pos = crq->request->sector + crq->request->nr_sectors;
crq->in_flight = 1;
crq->requeued = 0;
cfqq->in_flight++;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes
2005-05-24 16:35 [PATCH Linux 2.6.12-rc4-mm2 00/03] cfq: various fixes Tejun Heo
` (2 preceding siblings ...)
2005-05-24 16:35 ` [PATCH Linux 2.6.12-rc4-mm2 03/03] cfq: remove serveral unused fields from cfq data structures Tejun Heo
@ 2005-05-25 7:34 ` Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2005-05-25 7:34 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel
On Wed, May 25 2005, Tejun Heo wrote:
> Hello, Jens.
>
> This patchset is various fixes to cfq. All patches are against
> 2.6.12-rc4-mm2.
>
> One thing that isn't fixed in this patchset but I think might be
> problematic is the priority of async queue. It seems that the async
> queue doesn't receive any special attention regarding its priority.
> It just follows normal rules and its priority jumps irregularly.
> If I'm missing something, plz point out.
>
> [ Start of patch descriptions ]
>
> 01_cfq_INSERT_BACK_fix.patch
> : cfq ELEVATOR_INSERT_BACK fix
>
> When inserting INSERT_BACK request, cfq_insert_request() calls
> cfq_dispatch_requests() repetitively until it returns zero
> indicating no request is dispatched. This used to flush all
> the requests in the queue to the dispatch queue but, with idle
> slice implemented, the current active queue may decide to wait
> for new request using slice_timer. When this happens, 0 is
> returned from cfq_dispatch_requests() even when other cfqq's
> have pending requests. This breaks INSRET_BACK semantics.
>
> This patch adds @force argument which, when set to non-zero,
> disables idle_slice, and uses the argument when flushing
> cfqq's for INSERT_BACK. While at it, use INT_MAX instead of
> cfq_quantum when flushing cfqq's, as we're gonna dump all the
> requests and using cfq_quantum does nothing but adding
> unnecessary iterations.
>
> 02_cfq_ioc_leak_fix.patch
> : cfq_io_context leak fix
>
> When a process has more than one cic's associated with it,
> only the first one was kmem_cache_free'd in the original code.
> This patch frees all cic's in cfq_free_io_context().
>
> While at it, remove unnecessary refcounting from cic's to ioc.
> This reference is created when each cic is created and removed
> altogether when the ioc is exited, and, thus, serves no
> purpose.
>
> 03_cfq_remove_unused_fields.patch
> : remove serveral unused fields from cfq data structures
>
> cfq_data->idle_start, cfq_data->end_prio and cfq_rq->end_pos
> are not used in meaningful way. Remove'em.
All three patches look good, thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread