Re: [PATCH][noop-iosched] don't reuse a freed request

[Tejun Heo <htejun@gmail.com>]

Hi, guys.

Jens Axboe wrote:
> On Mon, Oct 31 2005, Arnaldo Carvalho de Melo wrote:
>
> > Hi,
> >
> > 	I'm getting the oops below when trying to use qemu with a kernel
> > built with just the noop iosched, I'm never had looked at this code before,
> > so I did a quick hack that seems enough for my case.
> >
> > 	Ah, this is with a fairly recent git tree (today), haven't checked
> > if it is present in 2.6.14.
> >
> > Best Regards,
> >
> > - Arnaldo
> >
> > Unable to handle kernel paging request at virtual address c5f20f60
> > printing eip:
> > c01b0ecd
> > *pde = 00017067
> > *pte = 05f20000
> > Oops: 0000 [#1]
> > DEBUG_PAGEALLOC
> > Modules linked in:
> > CPU:    0
> > EIP:    0060:[<c01b0ecd>]    Not tainted VLI
> > EFLAGS: 00000046   (2.6.14acme)
> > EIP is at elv_rq_merge_ok+0x15/0x7b
> > eax: 00000014   ebx: c5f20f58   ecx: 000003f8   edx: 00000046
> > esi: c12a5a90   edi: c5f20f58   ebp: c11658d0   esp: c11658c4
> > ds: 007b   es: 007b   ss: 0068
> > Process swapper (pid: 1, threadinfo=c1165000 task=c1164af0)
> > Stack: c0251883 c5ecfe4c c5d688c0 c1165904 c01b0f48 c5f20f58 c12a5a90 00000000
> >       c5874000 c018c5e1 c5f15f24 0000002b 00000000 c5ecfe4c c5d688c0 c12a5a90
> >       c1165920 c01b128d c5f20f58 c12a5a90 000a568a 00000000 00000002 c1165960
> > Call Trace:
> > [<c0102a63>] show_stack+0x78/0x83
> > [<c0102b88>] show_registers+0x100/0x167
> > [<c0102d35>] die+0xcb/0x140
> > [<c0234308>] do_page_fault+0x393/0x53a
> > [<c0102777>] error_code+0x4f/0x54
> > [<c01b0f48>] elv_try_merge+0x15/0x84
> > [<c01b128d>] elv_merge+0x1d/0x4f
> > [<c01b41d9>] __make_request+0xb2/0x425
> > [<c01b46f9>] generic_make_request+0x125/0x137
>
>
> Hrmpf, this looks really bad. Tejun, clearly there are still paths where
> ->last_rq isn't being cleared.

I'm currently debugging this.  The problem is that we are using generic 
dispatch queue directly in the noop and merging is NOT allowed on 
dispatch queues but generic handling of last_merge tries to merge 
requests.  I'm still trying to verify this, so I'll be back with results 
soon.

> > --- a/drivers/block/ll_rw_blk.c
> > +++ b/drivers/block/ll_rw_blk.c
> > @@ -1787,6 +1787,9 @@ static inline void blk_free_request(requ
> > 	if (rq->flags & REQ_ELVPRIV)
> > 		elv_put_request(q, rq);
> > 	mempool_free(rq, q->rq.rq_pool);
> > +
> > +	if (rq == q->last_merge)
> > +		q->last_merge = NULL;
> > }
> >
> > static inline struct request *
>
>
> It's most likely a bug getting this far in the first place, but does it
> fix things for you? I'll get on this asap.

If the bug is where I think it is, I think the proper thing to do is to 
use separate list_head in noop instead of using generic dispatch queue 
directly thus making noop consistent with other ioscheds.

I'm more worried about oops w/ cfq Arnaldo reported in this thread. 
I'll track that down as soon as I'm done with this one.

Many bugs.  Sorry. :-)

--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/