From: Alan Stern on
On Wed, 7 Apr 2010, Daniel Mack wrote:

> Ok, I'll write some dummies for usb_malloc() and usb_zalloc() which
> will just call kmalloc() with GFP_DMA32 for now. And while at it,
> usb_alloc_buffer() will be renamed to usb_alloc_consistent(). Then I'll
> try to clean up all existing drivers to use this new interface and
> follow the changes.
>
> In a next step, we should fine-tune when GFP_DMA32 is really needed.
> And I'll leave all occurances of usb_alloc_consistent() as they are now.
>
> Does that sound ok?

Yes, that should work out well.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oliver Neukum on
Am Mittwoch, 7. April 2010 16:59:47 schrieb Alan Stern:
> > The fix is to use usb_buffer_alloc() for that purpose which ensures
> > memory that is suitable for DMA. And on x86_64, this also means that the
> > upper 32 bits of the address returned are all 0's.
>
> That is not a good fix. usb_buffer_alloc() provides coherent memory,
> which is not what we want. I believe the correct fix is to specify the
> GFP_DMA32 flag in the kzalloc() call.
>
> Of course, some EHCI hardware is capable of using 64-bit addresses.
> But not all, and other controller types aren't. In principle we could
> create a new allocation routine, which would take a pointer to the USB
> bus as an additional argument and use it to decide whether the memory
> needs to lie below 4 GB. I'm not sure adding this extra complexity
> would be worthwhile.

What about XHCI? Do you really want to limit it to 32bits?

Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on
On Wed, Apr 07, 2010 at 06:54:55PM +0200, Oliver Neukum wrote:
> Am Mittwoch, 7. April 2010 16:59:47 schrieb Alan Stern:
> > > The fix is to use usb_buffer_alloc() for that purpose which ensures
> > > memory that is suitable for DMA. And on x86_64, this also means that the
> > > upper 32 bits of the address returned are all 0's.
> >
> > That is not a good fix. usb_buffer_alloc() provides coherent memory,
> > which is not what we want. I believe the correct fix is to specify the
> > GFP_DMA32 flag in the kzalloc() call.
> >
> > Of course, some EHCI hardware is capable of using 64-bit addresses.
> > But not all, and other controller types aren't. In principle we could
> > create a new allocation routine, which would take a pointer to the USB
> > bus as an additional argument and use it to decide whether the memory
> > needs to lie below 4 GB. I'm not sure adding this extra complexity
> > would be worthwhile.
>
> What about XHCI? Do you really want to limit it to 32bits?

No. Once we have the abstraction functions, we can well decide what to
do in there, depending on the actual controller we're running on.

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Mack on
On Wed, Apr 07, 2010 at 07:55:20PM +0200, Takashi Iwai wrote:
> At Wed, 7 Apr 2010 18:16:03 +0200,
> Daniel Mack wrote:
> >
> > On Wed, Apr 07, 2010 at 11:55:19AM -0400, Alan Stern wrote:
> > > On Wed, 7 Apr 2010, Greg KH wrote:
> > >
> > > > Alan, any objection to just using usb_buffer_alloc() for every driver?
> > > > Or is that too much overhead?
> > >
> > > I don't know what the overhead is. But usb_buffer_alloc() requires the
> > > caller to keep track of the buffer's DMA address, so it's not a simple
> > > plug-in replacement. In addition, the consistent memory that
> > > usb_buffer_alloc() provides is a scarce resource on some platforms.
> > >
> > > Writing new functions is the way to go.
> >
> > Ok, I'll write some dummies for usb_malloc() and usb_zalloc() which
> > will just call kmalloc() with GFP_DMA32 for now.
>
> Can't we provide only zalloc() variant? Zero'ing doesn't cost much,
> and the buffer allocation shouldn't be called too often.
>
> > And while at it,
> > usb_alloc_buffer() will be renamed to usb_alloc_consistent().
>
> Most of recent functions are named with "coherent".

I agree to both points, will do so unless anyone has a harsh opinion
about that.

Another thing: I guess we don't need a corresponding free() function
that just calls kfree(), right? Or should we introduce it now to be
flexible for future extensions?

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Takashi Iwai on
At Wed, 7 Apr 2010 18:16:03 +0200,
Daniel Mack wrote:
>
> On Wed, Apr 07, 2010 at 11:55:19AM -0400, Alan Stern wrote:
> > On Wed, 7 Apr 2010, Greg KH wrote:
> >
> > > Alan, any objection to just using usb_buffer_alloc() for every driver?
> > > Or is that too much overhead?
> >
> > I don't know what the overhead is. But usb_buffer_alloc() requires the
> > caller to keep track of the buffer's DMA address, so it's not a simple
> > plug-in replacement. In addition, the consistent memory that
> > usb_buffer_alloc() provides is a scarce resource on some platforms.
> >
> > Writing new functions is the way to go.
>
> Ok, I'll write some dummies for usb_malloc() and usb_zalloc() which
> will just call kmalloc() with GFP_DMA32 for now.

Can't we provide only zalloc() variant? Zero'ing doesn't cost much,
and the buffer allocation shouldn't be called too often.

> And while at it,
> usb_alloc_buffer() will be renamed to usb_alloc_consistent().

Most of recent functions are named with "coherent".


thanks,

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