FUJITA Tomonori [fujita.tomonori@xxxxxxxxxxxxx] wrote:
> On Tue, 28 Oct 2008 11:22:43 -0700
> malahal@xxxxxxxxxx wrote:
>
> > Now the rq->deadline can't be zero if the request is in the
> > timeout_list, so there is no need to have next_set. There is no need to
> > access a request's deadline field if blk_rq_timed_out is called on it.
> >
> > Signed-off-by: Malahal Naineni <malahal@xxxxxxxxxx>
> >
> > diff -r a6ae42397ede block/blk-timeout.c
> > --- a/block/blk-timeout.c Thu Oct 23 11:48:45 2008 -0700
> > +++ b/block/blk-timeout.c Fri Oct 24 17:08:24 2008 -0700
> > @@ -118,7 +118,7 @@
> > void blk_rq_timed_out_timer(unsigned long data)
> > {
> > struct request_queue *q = (struct request_queue *) data;
> > - unsigned long flags, uninitialized_var(next), next_set = 0;
> > + unsigned long flags, next = 0;
> > struct request *rq, *tmp;
> >
> > spin_lock_irqsave(q->queue_lock, flags);
> > @@ -133,15 +133,13 @@
> > if (blk_mark_rq_complete(rq))
> > continue;
> > blk_rq_timed_out(rq);
> > + } else {
> > + if (!next || time_after(next, rq->deadline))
> > + next = rq->deadline;
>
> Hmm, blk_rq_timed_out(rq) could put the rq back to q->timeout_list. We
> need to take care of the rq->deadline like this?
If it is put back on the list, it would be placed at the end of the list
so we will get to work on it again in the list traversal. There is no
need to access it now.
>
> > if (blk_mark_rq_complete(rq))
> > continue;
> > blk_rq_timed_out(rq);
> > + }
> >
> > + if (!next || time_after(next, rq->deadline))
> > + next = rq->deadline;
>
> But if blk_rq_timed_out calls __blk_complete_request, we don't want to
> touch the rq here, I guess.
Yes, we should not access it although I don't think the request can be
freed while the existing code is accessing the deadline field. What
likely can happen is that we may call mod_timer with jiffies that is
older than current which would call the timer immediately...
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
