| Message ID | 20180505082208.32553-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/05/18 09:22, Hans de Goede wrote: > The recent "x86 ZONE_DMA love" discussion at LSF/MM pointed out that some > gspca sub-drivvers are using GFP_DMA to allocate buffers which are used > for USB bulk transfers, there is absolutely no need for this, drop it. > The documentation for kmalloc() says GFP_DMA - Allocation suitable for DMA. end at least in sq905.c the allocation is passed to the USB stack that then uses it for DMA. Looking a bit closer the "suitable for DMA" label that GFP_DMA promises is not really a sensible thing for kmalloc() to determine as it is dependent on the DMA controller in question. The USB stack now ensures that everything works correctly as long as the memory is allocated with kmalloc() so acked by me for sq905.c but, is anyone taking care of fixing the kmalloc() documentation? Adam Baker
Hi, On 05/13/2018 07:54 PM, Adam Baker wrote: > On 05/05/18 09:22, Hans de Goede wrote: >> The recent "x86 ZONE_DMA love" discussion at LSF/MM pointed out that some >> gspca sub-drivvers are using GFP_DMA to allocate buffers which are used >> for USB bulk transfers, there is absolutely no need for this, drop it. >> > > The documentation for kmalloc() says > GFP_DMA - Allocation suitable for DMA. > > end at least in sq905.c the allocation is passed to the USB stack that > then uses it for DMA. > > Looking a bit closer the "suitable for DMA" label that GFP_DMA promises > is not really a sensible thing for kmalloc() to determine as it is > dependent on the DMA controller in question. The USB stack now ensures > that everything works correctly as long as the memory is allocated with > kmalloc() so acked by me for sq905.c but, is anyone taking care of > fixing the kmalloc() documentation? The whole GFP_DMA flag use in the kernel is a mess and fixing the doucmentation is not easy and likely also not the solution, see: https://lwn.net/Articles/753273/ Note this article is currently only available to LWN subscribers (it will become freely available in a week). I'll send you a private mail with a link which will allow you to read it. Regards, Hans
diff --git a/drivers/media/usb/gspca/jl2005bcd.c b/drivers/media/usb/gspca/jl2005bcd.c index d668589598d6..c40245950553 100644 --- a/drivers/media/usb/gspca/jl2005bcd.c +++ b/drivers/media/usb/gspca/jl2005bcd.c @@ -321,7 +321,7 @@ static void jl2005c_dostream(struct work_struct *work) int ret; u8 *buffer; - buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA); + buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL); if (!buffer) { pr_err("Couldn't allocate USB buffer\n"); goto quit_stream; diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c index cc8ff41b8ab3..ffea9c35b0a0 100644 --- a/drivers/media/usb/gspca/sq905.c +++ b/drivers/media/usb/gspca/sq905.c @@ -217,7 +217,7 @@ static void sq905_dostream(struct work_struct *work) u8 *data; u8 *buffer; - buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL | GFP_DMA); + buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL); if (!buffer) { pr_err("Couldn't allocate USB buffer\n"); goto quit_stream; diff --git a/drivers/media/usb/gspca/sq905c.c b/drivers/media/usb/gspca/sq905c.c index 5e1269eb7c50..274921c0bb46 100644 --- a/drivers/media/usb/gspca/sq905c.c +++ b/drivers/media/usb/gspca/sq905c.c @@ -138,7 +138,7 @@ static void sq905c_dostream(struct work_struct *work) int ret; u8 *buffer; - buffer = kmalloc(SQ905C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA); + buffer = kmalloc(SQ905C_MAX_TRANSFER, GFP_KERNEL); if (!buffer) { pr_err("Couldn't allocate USB buffer\n"); goto quit_stream; diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c index 554b90ef2200..8562bda0ef88 100644 --- a/drivers/media/usb/gspca/vicam.c +++ b/drivers/media/usb/gspca/vicam.c @@ -182,7 +182,7 @@ static void vicam_dostream(struct work_struct *work) frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage + HEADER_SIZE; - buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA); + buffer = kmalloc(frame_sz, GFP_KERNEL); if (!buffer) { pr_err("Couldn't allocate USB buffer\n"); goto exit;
The recent "x86 ZONE_DMA love" discussion at LSF/MM pointed out that some gspca sub-drivvers are using GFP_DMA to allocate buffers which are used for USB bulk transfers, there is absolutely no need for this, drop it. Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/usb/gspca/jl2005bcd.c | 2 +- drivers/media/usb/gspca/sq905.c | 2 +- drivers/media/usb/gspca/sq905c.c | 2 +- drivers/media/usb/gspca/vicam.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)