Messages in this thread |  | | | Date | Mon, 31 Oct 2005 17:59:18 +0900 | | From | Tejun Heo <> | | Subject | Re: [PATCH][noop-iosched] don't reuse a freed request |
| |
Jens Axboe wrote: > On Mon, Oct 31 2005, Tejun Heo wrote: > >>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. > > > So either we disable merging for noop by setting REQ_NOMERGE in > elevator_noop_add_request(), or we add a noop_list and do the > dispatching like in the other io schedulers. I'd prefer the latter, > merging is still beneficial for noop (and it has always done it).
Just verified. It happens when elv_merge_requests() happens. The merged request should be unlinked from list but noop does not have any merge handling callbacks ATM.
Sorry about the hassle. :-(
> > For now, we should add the former.
Yeap, also verified oops doesn't happen with the following patch.
I'll soon post a patch to convert noop such that it does proper dispatching. BTW, while I was looking at the code, I found something else, in elv_former/latter_request functions, if the iosched doesn't supply the callbacks, it uses rq->queue_list.prev/next implicitly (without this, this noop bug wouldn't have been triggered). I think this code is not necessary anymore. What do you think?
Thanks.
-- 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/
|  |