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.
[ End of patch descriptions ]
Thanks.
--
tejun
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 <[email protected]>
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);
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 <[email protected]>
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;
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 <[email protected]>
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++;
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