Re: Badness in as_completed_request warning

Nick Piggin
2003-09-09T07:04:27Z

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

Dave Olien wrote:

>On Tue, Sep 09, 2003 at 01:40:57PM +1000, Nick Piggin wrote:
>
>>Hi Dave,
>>Can you try 2.6.0-test5 with the following patch please. Thanks
>>
>>
>
>I applied this patch, and gave it a shot. It seems to have made things
>worse. Instead of the warnings occurring only at the end of the mkfs
>runs, they now occur pretty much constantly, and it finally ends
>with an Bug_On. Attached is the console output. There are 20
>occurrances of Badness in as-iosched.c line 1020. There is one
>occurrance of Badness in as-iosched.c line 1025. The Bug_On() is in
>as-iosched.c line 1243.
>
>Console output is attached.
>

Thanks Dave,
Can you try this one? I can't reproduce the problem here as I don't have
enough disks unfortunately. Thanks

Nick

[-- Attachment #2: as-warn-fix.patch --]
[-- Type: text/plain, Size: 3561 bytes --]

linux-2.6-npiggin/drivers/block/as-iosched.c | 60 +++++++++++++++++++--------
1 files changed, 43 insertions(+), 17 deletions(-)

diff -puN drivers/block/as-iosched.c~as-warn-fix drivers/block/as-iosched.c
--- linux-2.6/drivers/block/as-iosched.c~as-warn-fix 2003-09-09 17:02:41.000000000 +1000
+++ linux-2.6-npiggin/drivers/block/as-iosched.c 2003-09-09 17:02:41.000000000 +1000
@@ -373,7 +373,7 @@ static void as_move_to_dispatch(struct a
* direct IO, then move the alias to the dispatch list and then add the
* request.
*/
-static void as_add_arq_rb(struct as_data *ad, struct as_rq *arq)
+static int as_add_arq_rb(struct as_data *ad, struct as_rq *arq)
{
struct as_rq *alias;
struct request *rq = arq->request;
@@ -381,10 +381,20 @@ static void as_add_arq_rb(struct as_data
arq->rb_key = rq_rb_key(rq);

/* This can be caused by direct IO */
+#if 0
while ((alias = __as_add_arq_rb(ad, arq)))
as_move_to_dispatch(ad, alias);
+#endif
+ if ((alias = __as_add_arq_rb(ad, arq))) {
+ arq->state = AS_RQ_NEW;
+ list_add_tail(&arq->request->queuelist,
+ &alias->request->queuelist);
+
+ return 1;
+ }

rb_insert_color(&arq->rb_node, ARQ_RB_ROOT(ad, arq));
+ return 0;
}

static inline void as_del_arq_rb(struct as_data *ad, struct as_rq *arq)
@@ -1131,7 +1141,21 @@ static void as_move_to_dispatch(struct a
* take it off the sort and fifo list, add to dispatch queue
*/
as_remove_queued_request(ad->q, arq->request);
+
+ for (;;) {
+ struct list_head *alias = &arq->request->queuelist;
+
+ if (!list_empty(alias)) {
+ struct request *rq;
+ rq = list_entry_rq(alias->next);
+ list_del(&rq->queuelist);
+ list_add_tail(&rq->queuelist, ad->dispatch);
+ } else
+ break;
+ }
+
list_add_tail(&arq->request->queuelist, ad->dispatch);
+
if (arq->io_context && arq->io_context->aic)
atomic_inc(&arq->io_context->aic->nr_dispatched);

@@ -1303,22 +1327,22 @@ static void as_add_request(struct as_dat
arq->is_sync = 0;
data_dir = arq->is_sync;

- arq->io_context = as_get_io_context();
+ if (!as_add_arq_rb(ad, arq)) {
+ arq->io_context = as_get_io_context();

- if (arq->io_context) {
- atomic_inc(&arq->io_context->aic->nr_queued);
- as_update_iohist(arq->io_context->aic, arq->request);
- }
-
- as_add_arq_rb(ad, arq);
+ if (arq->io_context) {
+ atomic_inc(&arq->io_context->aic->nr_queued);
+ as_update_iohist(arq->io_context->aic, arq->request);
+ }

- /*
- * set expire time (only used for reads) and add to fifo list
- */
- arq->expires = jiffies + ad->fifo_expire[data_dir];
- list_add_tail(&arq->fifo, &ad->fifo_list[data_dir]);
- arq->state = AS_RQ_QUEUED;
- as_update_arq(ad, arq); /* keep state machine up to date */
+ /*
+ * set expire time (only used for reads) and add to fifo list
+ */
+ arq->expires = jiffies + ad->fifo_expire[data_dir];
+ list_add_tail(&arq->fifo, &ad->fifo_list[data_dir]);
+ arq->state = AS_RQ_QUEUED;
+ as_update_arq(ad, arq); /* keep state machine up to date */
+ }
}

/*
@@ -1495,7 +1519,8 @@ static void as_merged_request(request_qu
*/
if (rq_rb_key(req) != arq->rb_key) {
as_del_arq_rb(ad, arq);
- as_add_arq_rb(ad, arq);
+ if (as_add_arq_rb(ad, arq))
+ WARN_ON(1);
/*
* Note! At this stage of this and the next function, our next
* request may not be optimal - eg the request may have "grown"
@@ -1526,7 +1551,8 @@ as_merged_requests(request_queue_t *q, s

if (rq_rb_key(req) != arq->rb_key) {
as_del_arq_rb(ad, arq);
- as_add_arq_rb(ad, arq);
+ if (as_add_arq_rb(ad, arq))
+ WARN_ON(1);
}

/*

_