On Mon, 2008-02-25 at 15:56 +0100, Andi Kleen wrote: > On Mon, Feb 25, 2008 at 06:43:08AM -0800, James Bottomley wrote: > > On Mon, 2008-02-25 at 00:36 +0100, Andi Kleen wrote: > > > Don't use unnecessary GFP_ATOMIC in sg.c > > > > > > sg.c uses GFP_ATOMIC for a lot of allocations where it isn't necessary. > > > There are no locks hold and I don't see any other reasons why GFP_ATOMIC > > > should be needed here. So remove them for more reliable allocations. > > > > > > Depends on the earlier GFP_DMA patchkit, but only because it happens > > > to patch the same places (no real functional dependency) > > > > > > Tested by burning a CD on a kernel with all lock/sleep/etc. debugging enabled. > > > > I'm afraid this is wrong. Those paths are used in the block layer issue > > path. Most of the time this path does have user context. However, it's > > legal to call it from tasklet context, and most error retries do this. > > In that case, your GFP_KERNEL will have problems, so these have to be > > GFP_ATOMIC, I'm afraid. > > In what path? I couldn't find one while reading the code. But I don't > claim to understand it completely so I might have missed something. It's a fundamental lack of guarantee of the API. Block *may* give you user context but doesn't guarantee it. It's so we can call blk_run_queue from the block tasklet. This is usually done by queue restarts. SCSI does it via scsi_next_command, which is usually in tasklet context. You'll really only see this for a TCQ device (which no CDs are). James - 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
