Messages in this thread | | | Date | Sun, 18 May 2008 20:23:03 -0600 | From | Robert Hancock <> | Subject | Re: Top kernel oopses/warnings for the week of May 16th 2008 |
| |
Andrea Arcangeli wrote: > On Sat, May 17, 2008 at 07:57:26AM -0700, Arjan van de Ven wrote: >> Andrea Arcangeli wrote: >>> The reason I touched that code, is that a change introduced during >>> 2.6.25-rc initialized the isa dma pool even if not necessary and that >>> broke the reserved-ram patch that requires no __GFP_DMA >>> allocations. There was no crash in 2.6.24 based kernels, the >>> regression started in 2.6.25-rc. >> I'd not really call "breaks external patch" a regression ;) > > The external patch only allowed me to notice the regression when > nobody else noticed it. For mainline the regression was to put ram > into the bounce buffer pool even if no dma could ever require the > bounce buffering. There's no point to initialize the pool when total > ram < highest dma address. That is the regression. My patch turned the > regression from a waste of ram, to a kernel crash at boot. That's the > only relation between the reserved-ram patch this bug. > > I assume Robert has a similar issue with some debugging code checking > for GFP_KERNEL allocations inside atomic context or similar, I assume > his driver has a bug and calls that function in the wrong context. But > if this didn't happen in 2.4.24, it means such bug has nothing to do > with the bug in blk-settings.c. It's just that such a bug or the > reserved-ram patches are required to notice the regression in > blk-settings.c.
Well, it's not really documented what the locking semantics are supposed to be for blk_queue_bounce_limit. Based on the implementation, though, it's OK to call it under spin_lock_irqsave (it only sets some variables) unless you hit the case where dma is set to 1 and we do init_emergency_isa_pool. That's the problem, that case should not be hit with a DMA mask of 32-bit, but with Yang Shi's change to blk-settings.c, now we are.
The code in that function seems rather hackish, actually. It seems like a lot of those assumptions it's making should be in architecture-specific code..
| |