Re: [PATCH] as i/o hang with aacraid driver 2.6.0-test1

Mark Haverkamp
2003-07-18T14:48:37Z

On Thu, 2003-07-17 at 22:14, Nick Piggin wrote:
> Andrew Morton wrote:
>
> >Daniel McNeil <daniel@osdl.org> wrote:
> >
> >>Should
> >> list_add(&rq->queuelist, ad->dispatch->prev);
> >>be
> >> list_add_tail(&rq->queuelist, ad->dispatch);
> >>
> >>It is equivalent, but makes the code clearer that we're adding to the
> >>end.
> >>
> >
> >yup.
> >
> >
> >>Also, isn't the: !list_empty(ad->dispatch) unnecessary since a
> >>list_add was just done, so the list cannot be empty?
> >>
> >
> >more yup.
> >
>
> Oh, so the request should go on the dispatch list?
> OK that makes sense. Maybe also set arq->state = AS_RQ_DISPATCHED
> just in case.
>
> And I think the arq == NULL case should be checked for with
> rq->flags & REQ_HARDBARRIER || !blk_fs_request(rq) for consistency.
> I think if arq is still NULL then its a bug.
>
> Thanks
> Nick

Nick,

I tried what you suggested and I'm pretty sure that aic->nr_dispatched
is not getting incremented when it should be. I get:

Badness in as_remove_dispatched_request at drivers/block/as-iosched.c:1019

Which says that aic->nr_dispatched is wrong. I have attached the
as_requeue_request that I was using. Is this what you had in mind?

Thanks,
Mark.

---------------

/*
* requeue the request. The request has not been completed, nor is it a
* new request, so don't touch accounting.
*/
static void as_requeue_request(request_queue_t *q, struct request *rq)
{
struct as_data *ad = q->elevator.elevator_data;
struct as_rq *arq = RQ_DATA(rq);

if (unlikely(rq->flags & REQ_HARDBARRIER) ||
unlikely(!blk_fs_request(rq))) {
BUG_ON(!arq);
arq->state = AS_RQ_DISPATCHED;
if (arq->io_context && arq->io_context->aic)
atomic_inc(&arq->io_context->aic->nr_dispatched);
}

list_add_tail(&rq->queuelist, ad->dispatch);

/* Stop anticipating - let this request get through */
if (ad->antic_status == ANTIC_WAIT_REQ
|| ad->antic_status == ANTIC_WAIT_NEXT)
as_antic_stop(ad);

return;
}

--
Mark Haverkamp <markh@osdl.org>