[PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers

[PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers

[Hans de Goede]

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(-)

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;
-- 
2.17.0

Re: [PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers

[Adam Baker]

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

Re: [PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers

[Hans de Goede]

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