Re: [PATCH 0/4] revert the commit 22a9189f (cdrom: use kmalloced buffers instead of buffers on stack)

[FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>]

On Thu, 22 May 2008 00:11:13 +0200
tsbogend@alpha.franken.de (Thomas Bogendoerfer) wrote:

> On Tue, May 20, 2008 at 01:58:30PM +0900, FUJITA Tomonori wrote:
> > The goal of this patchset is reverting the commit
> > 22a9189fd073db3d03a4cf8b8c098aa207602de1 (cdrom: use kmalloced buffers
> > instead of buffers on stack).
> > 
> > http://lkml.org/lkml/2008/4/21/634
> > 
> > The commit is using kmalloced buffers for cdrom packet commands to
> > avoid stack corruption on non coherent platforms. But allocating a
> > small buffer like this is not nice (unnecessary complicity):
> > 
> > +	buffer = kmalloc(8, GFP_KERNEL);
> 
> If I read the bio code correctly with your patch non coherent system will
> be punished a lot more. bio_copy_kern will allocate a whole page for this
> 8 bytes. Please correct me, if I'm wrong otherwise I tend to NAK that 
> approach.

You read the block layer code correctly, but I don't think allocating
one page frame is expensive.

We still have many places to do DMA on stack and need to fix them.

There are still many places (scsi_execute users) in the scsi mid-layer
that uses stack addresses for DMA.

drivers/block/pktcdvd.c always uses stack addresses for DMA for
generic packets.

I don't think that using kmalloc/kfree all over the place is a good
idea. We have a mechanism to do I/Os via a temporary buffer for a
buffer that we can't do DMA against. This patch just uses the mechnism
instead of spreading kmalloc/kfree everywhere. Note that of course
this patch fixes the problems that I explained above.


> > If some architectures can't do DMA on stack, we also need to a new
> > queue_flag like QUEUE_FLAG_NO_DMA_ON_STACK in addtion of this
> > patchset.
> 
> DMA-mapping.txt tells that DMA on stack is forbidden. IMHO we either need
> to revoke that first or deal with it. 

Yeah, it would be better to postpone the 4th patch until we finish
this issue. But I guess that we have enough time to do that until the
next merge window.

DMA on stack is forbidden because of non coherent architecutes and
architectures can't uses stack addresses for DMA? What architectures
can't uses stack addresses for DMA? Would it be better to just forbid
using stack addresses for DMA on all the architectures at all times?