[RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Jaewon Kim]

Hello I don't have enough knowledge on USB core but I've wondered
why GFP_NOIO has been used in xhci_alloc_dev for
xhci_alloc_virt_device. I found commit ("a6d940dd759b xhci: Use
GFP_NOIO during device reset"). But can we just change GFP_NOIO
to __GFP_RECLAIM | __GFP_FS ?

Please refer to below case.

I got a report from Lee YongTaek <ytk.lee@samsung.com> that the
xhci_alloc_virt_device was too slow over 2 seconds only for one page
allocation.

1) It was because kernel version was v4.14 and DMA allocation was
done from CMA(Contiguous Memory Allocator) where CMA region was
almost filled with file page and  CMA passes GFP down to page
isolation. And the page isolation only allows file page isolation only to
requests having __GFP_FS.

2) Historically CMA was changed at v4.19 to use GFP_KERNEL
regardless of GFP passed to  DMA allocation through the
commit 6518202970c1 "(mm/cma: remove unsupported gfp_mask
parameter from cma_alloc()".

I think pre v4.19 the xhci_alloc_virt_device could be very slow
depending on CMA situation but free to USB deadlock issue. But as of
v4.19, I think, it will be fast but can face the deadlock issue.
Consequently I think to meet the both cases, I think USB can pass
__GFP_FS without __GFP_IO.

If __GFP_FS is passed from USB core, of course, the CMA patch also
need to be changed to pass GFP.

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 005e65922608..38abcd03a1a2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3893,7 +3893,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct
usb_device *udev)
         * xhci_discover_or_reset_device(), which may be called as part of
         * mass storage driver error handling.
         */
-       if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
+       if (!xhci_alloc_virt_device(xhci, slot_id, udev, __GFP_RECLAIM
| __GFP_FS)) {
                xhci_warn(xhci, "Could not allocate xHCI USB device
data structures\n");
                goto disable_slot;
        }


Thank you

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Matthew Wilcox]

On Sat, May 18, 2019 at 01:02:28AM +0900, Jaewon Kim wrote:
> Hello I don't have enough knowledge on USB core but I've wondered
> why GFP_NOIO has been used in xhci_alloc_dev for
> xhci_alloc_virt_device. I found commit ("a6d940dd759b xhci: Use
> GFP_NOIO during device reset"). But can we just change GFP_NOIO
> to __GFP_RECLAIM | __GFP_FS ?

No.  __GFP_FS implies __GFP_IO; you can't set __GFP_FS and clear __GFP_IO.

It seems like the problem you have is using the CMA to do DMA allocation.
Why would you do such a thing?

> Please refer to below case.
> 
> I got a report from Lee YongTaek <ytk.lee@samsung.com> that the
> xhci_alloc_virt_device was too slow over 2 seconds only for one page
> allocation.
> 
> 1) It was because kernel version was v4.14 and DMA allocation was
> done from CMA(Contiguous Memory Allocator) where CMA region was
> almost filled with file page and  CMA passes GFP down to page
> isolation. And the page isolation only allows file page isolation only to
> requests having __GFP_FS.
> 
> 2) Historically CMA was changed at v4.19 to use GFP_KERNEL
> regardless of GFP passed to  DMA allocation through the
> commit 6518202970c1 "(mm/cma: remove unsupported gfp_mask
> parameter from cma_alloc()".
> 
> I think pre v4.19 the xhci_alloc_virt_device could be very slow
> depending on CMA situation but free to USB deadlock issue. But as of
> v4.19, I think, it will be fast but can face the deadlock issue.
> Consequently I think to meet the both cases, I think USB can pass
> __GFP_FS without __GFP_IO.
> 
> If __GFP_FS is passed from USB core, of course, the CMA patch also
> need to be changed to pass GFP.

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Jaewon Kim]

Thank you for your comment.

In ARM64 architecture, default CMA region is commonly activated and it
could be used
if no specific memory region is defined. The USB driver in my platform
also uses the
default CMA region as DMA allocation. If using the CMA to do DMA
allocation is improper,
then do you think that the USB driver for my platform should be
changed not to use CMA?

According to my understanding, in CONFIG_DMA_CMA on both v4.14 and v5.0,
__GFP_DIRECT_RECLAIM will try CMA allocation first instead of normal
buddy allocation.
Then it will get default CMA region through dev_get_cma_area API.
Please refer to
dev_get_cma_area code below though I am using v4.14 for my platform.

git show v5.0:include/linux/dma-contiguous.h
 61 #ifdef CONFIG_DMA_CMA
 62
 63 extern struct cma *dma_contiguous_default_area;
 64
 65 static inline struct cma *dev_get_cma_area(struct device *dev)
 66 {
 67         if (dev && dev->cma_area)
 68                 return dev->cma_area;
 69         return dma_contiguous_default_area;
 70 }

Thank you

2019년 5월 18일 (토) 오전 1:34, Matthew Wilcox <willy@infradead.org>님이 작성:
>
> On Sat, May 18, 2019 at 01:02:28AM +0900, Jaewon Kim wrote:
> > Hello I don't have enough knowledge on USB core but I've wondered
> > why GFP_NOIO has been used in xhci_alloc_dev for
> > xhci_alloc_virt_device. I found commit ("a6d940dd759b xhci: Use
> > GFP_NOIO during device reset"). But can we just change GFP_NOIO
> > to __GFP_RECLAIM | __GFP_FS ?
>
> No.  __GFP_FS implies __GFP_IO; you can't set __GFP_FS and clear __GFP_IO.
>
> It seems like the problem you have is using the CMA to do DMA allocation.
> Why would you do such a thing?
>
> > Please refer to below case.
> >
> > I got a report from Lee YongTaek <ytk.lee@samsung.com> that the
> > xhci_alloc_virt_device was too slow over 2 seconds only for one page
> > allocation.
> >
> > 1) It was because kernel version was v4.14 and DMA allocation was
> > done from CMA(Contiguous Memory Allocator) where CMA region was
> > almost filled with file page and  CMA passes GFP down to page
> > isolation. And the page isolation only allows file page isolation only to
> > requests having __GFP_FS.
> >
> > 2) Historically CMA was changed at v4.19 to use GFP_KERNEL
> > regardless of GFP passed to  DMA allocation through the
> > commit 6518202970c1 "(mm/cma: remove unsupported gfp_mask
> > parameter from cma_alloc()".
> >
> > I think pre v4.19 the xhci_alloc_virt_device could be very slow
> > depending on CMA situation but free to USB deadlock issue. But as of
> > v4.19, I think, it will be fast but can face the deadlock issue.
> > Consequently I think to meet the both cases, I think USB can pass
> > __GFP_FS without __GFP_IO.
> >
> > If __GFP_FS is passed from USB core, of course, the CMA patch also
> > need to be changed to pass GFP.
>
>

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Christoph Hellwig]

Folks, you can't just pass arbitary GFP_ flags to dma allocation
routines, beause very often they are not just wrappers around
the page allocator.

So no, you can't just fine grained control the flags, but the
existing code is just as buggy.

Please switch to use memalloc_noio_save() instead.

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On So, 2019-05-19 at 22:56 -0700, Christoph Hellwig wrote:
> Folks, you can't just pass arbitary GFP_ flags to dma allocation
> routines, beause very often they are not just wrappers around
> the page allocator.
> 
> So no, you can't just fine grained control the flags, but the
> existing code is just as buggy.
> 
> Please switch to use memalloc_noio_save() instead.
> 

Hi,

we actually do. It is just higher up in the calling path:

int usb_reset_device(struct usb_device *udev)
{
        int ret;
        int i;
        unsigned int noio_flag;
        struct usb_port *port_dev;
        struct usb_host_config *config = udev->actconfig;
        struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);

        if (udev->state == USB_STATE_NOTATTACHED ||
                        udev->state == USB_STATE_SUSPENDED) {
                dev_dbg(&udev->dev, "device reset not allowed in state %d\n",
                                udev->state);
                return -EINVAL;
        }

        if (!udev->parent) {
                /* this requires hcd-specific logic; see ohci_restart() */
                dev_dbg(&udev->dev, "%s for root hub!\n", __func__);
                return -EISDIR;
        }

        port_dev = hub->ports[udev->portnum - 1];

        /*
         * Don't allocate memory with GFP_KERNEL in current
         * context to avoid possible deadlock if usb mass
         * storage interface or usbnet interface(iSCSI case)
         * is included in current configuration. The easist
         * approach is to do it for every device reset,
         * because the device 'memalloc_noio' flag may have
         * not been set before reseting the usb device.
         */
        noio_flag = memalloc_noio_save();

So, do we need to audit the mem_flags again?
What are we supposed to use? GFP_KERNEL?

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Christoph Hellwig]

On Mon, May 20, 2019 at 11:09:25AM +0200, Oliver Neukum wrote:
> we actually do. It is just higher up in the calling path:

Perfect!

> So, do we need to audit the mem_flags again?
> What are we supposed to use? GFP_KERNEL?

GFP_KERNEL if you can block, GFP_ATOMIC if you can't for a good reason,
that is the allocation is from irq context or under a spinlock.  If you
think you have a case where you think you don't want to block, but it
is not because of the above reasons we need to have a chat about the
details.

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Alan Stern]

On Mon, 20 May 2019, Christoph Hellwig wrote:

> On Mon, May 20, 2019 at 11:09:25AM +0200, Oliver Neukum wrote:
> > we actually do. It is just higher up in the calling path:
> 
> Perfect!
> 
> > So, do we need to audit the mem_flags again?
> > What are we supposed to use? GFP_KERNEL?
> 
> GFP_KERNEL if you can block, GFP_ATOMIC if you can't for a good reason,
> that is the allocation is from irq context or under a spinlock.  If you
> think you have a case where you think you don't want to block, but it
> is not because of the above reasons we need to have a chat about the
> details.

What if the allocation requires the kernel to swap some old pages out 
to the backing store, but the backing store is on the device that the 
driver is managing?  The swap can't take place until the current I/O 
operation is complete (assuming the driver can handle only one I/O 
operation at a time), and the current operation can't complete until 
the old pages are swapped out.  Result: deadlock.

Isn't that the whole reason for using GFP_NOIO in the first place?

Alan Stern

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Christoph Hellwig]

On Mon, May 20, 2019 at 10:16:57AM -0400, Alan Stern wrote:
> What if the allocation requires the kernel to swap some old pages out 
> to the backing store, but the backing store is on the device that the 
> driver is managing?  The swap can't take place until the current I/O 
> operation is complete (assuming the driver can handle only one I/O 
> operation at a time), and the current operation can't complete until 
> the old pages are swapped out.  Result: deadlock.
> 
> Isn't that the whole reason for using GFP_NOIO in the first place?

It is, or rather was.  As it has been incredibly painful to wire
up the gfp_t argument through some callstacks, most notably the
vmalloc allocator which is used by a lot of the DMA allocators on
non-coherent platforms, we now have the memalloc_noio_save and
memalloc_nofs_save functions that mark a thread as not beeing to
go into I/O / FS reclaim.  So even if you use GFP_KERNEL you will
not dip into reclaim with those flags set on the thread.

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On Mo, 2019-05-20 at 07:23 -0700, Christoph Hellwig wrote:
> On Mon, May 20, 2019 at 10:16:57AM -0400, Alan Stern wrote:
> > What if the allocation requires the kernel to swap some old pages out 
> > to the backing store, but the backing store is on the device that the 
> > driver is managing?  The swap can't take place until the current I/O 
> > operation is complete (assuming the driver can handle only one I/O 
> > operation at a time), and the current operation can't complete until 
> > the old pages are swapped out.  Result: deadlock.
> > 
> > Isn't that the whole reason for using GFP_NOIO in the first place?
> 
> It is, or rather was.  As it has been incredibly painful to wire
> up the gfp_t argument through some callstacks, most notably the
> vmalloc allocator which is used by a lot of the DMA allocators on
> non-coherent platforms, we now have the memalloc_noio_save and
> memalloc_nofs_save functions that mark a thread as not beeing to
> go into I/O / FS reclaim.  So even if you use GFP_KERNEL you will
> not dip into reclaim with those flags set on the thread.

OK, but this leaves a question open. Will the GFP_NOIO actually
hurt, if it is used after memalloc_noio_save()?

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Christoph Hellwig]

On Tue, May 21, 2019 at 10:54:37AM +0200, Oliver Neukum wrote:
> OK, but this leaves a question open. Will the GFP_NOIO actually
> hurt, if it is used after memalloc_noio_save()?

Unless we have a bug somewhere it should not make any difference,
neither positively nor negatively.

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On Mo, 2019-05-20 at 10:16 -0400, Alan Stern wrote:
> On Mon, 20 May 2019, Christoph Hellwig wrote:
> 
> > GFP_KERNEL if you can block, GFP_ATOMIC if you can't for a good reason,
> > that is the allocation is from irq context or under a spinlock.  If you
> > think you have a case where you think you don't want to block, but it
> > is not because of the above reasons we need to have a chat about the
> > details.
> 
> What if the allocation requires the kernel to swap some old pages out 
> to the backing store, but the backing store is on the device that the 
> driver is managing?  The swap can't take place until the current I/O 
> operation is complete (assuming the driver can handle only one I/O 
> operation at a time), and the current operation can't complete until 
> the old pages are swapped out.  Result: deadlock.
> 
> Isn't that the whole reason for using GFP_NOIO in the first place?

Hi,

lookig at this it seems to me that we are in danger of a deadlock

- during reset - devices cannot do IO while being reset
	covered by the USB layer in usb_reset_device
- resume & restore - devices cannot do IO while suspended
	covered by driver core in rpm_callback
- disconnect - a disconnected device cannot do IO
	is this a theoretical case or should I do something to
	the driver core?

How about changing configurations on USB?

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Alan Stern]

On Tue, 21 May 2019, Oliver Neukum wrote:

> On Mo, 2019-05-20 at 10:16 -0400, Alan Stern wrote:
> > On Mon, 20 May 2019, Christoph Hellwig wrote:
> > 
> > > GFP_KERNEL if you can block, GFP_ATOMIC if you can't for a good reason,
> > > that is the allocation is from irq context or under a spinlock.  If you
> > > think you have a case where you think you don't want to block, but it
> > > is not because of the above reasons we need to have a chat about the
> > > details.
> > 
> > What if the allocation requires the kernel to swap some old pages out 
> > to the backing store, but the backing store is on the device that the 
> > driver is managing?  The swap can't take place until the current I/O 
> > operation is complete (assuming the driver can handle only one I/O 
> > operation at a time), and the current operation can't complete until 
> > the old pages are swapped out.  Result: deadlock.
> > 
> > Isn't that the whole reason for using GFP_NOIO in the first place?
> 
> Hi,
> 
> lookig at this it seems to me that we are in danger of a deadlock
> 
> - during reset - devices cannot do IO while being reset
> 	covered by the USB layer in usb_reset_device
> - resume & restore - devices cannot do IO while suspended
> 	covered by driver core in rpm_callback
> - disconnect - a disconnected device cannot do IO
> 	is this a theoretical case or should I do something to
> 	the driver core?
> 
> How about changing configurations on USB?

Changing configurations amounts to much the same as disconnecting,
because both operations destroy all the existing interfaces.

Disconnect can arise in two different ways.

	Physical hot-unplug: All I/O operations will fail.

	Rmmod or unbind: I/O operations will succeed.

The second case is probably okay.  The first we can do nothing about.  
However, in either case we do need to make sure that memory allocations
do not require any writebacks.  This suggests that we need to call
memalloc_noio_save() from within usb_unbind_interface().

Alan Stern

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On Di, 2019-05-21 at 10:00 -0400, Alan Stern wrote:
> 
> Changing configurations amounts to much the same as disconnecting,
> because both operations destroy all the existing interfaces.
> 
> Disconnect can arise in two different ways.
> 
>         Physical hot-unplug: All I/O operations will fail.
> 
>         Rmmod or unbind: I/O operations will succeed.
> 
> The second case is probably okay.  The first we can do nothing about.  
> However, in either case we do need to make sure that memory allocations
> do not require any writebacks.  This suggests that we need to call
> memalloc_noio_save() from within usb_unbind_interface().

I agree with the problem, but I fail to see why this issue would be
specific to USB. Shouldn't this be done in the device core layer?

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Alan Stern]

On Wed, 22 May 2019, Oliver Neukum wrote:

> On Di, 2019-05-21 at 10:00 -0400, Alan Stern wrote:
> > 
> > Changing configurations amounts to much the same as disconnecting,
> > because both operations destroy all the existing interfaces.
> > 
> > Disconnect can arise in two different ways.
> > 
> >         Physical hot-unplug: All I/O operations will fail.
> > 
> >         Rmmod or unbind: I/O operations will succeed.
> > 
> > The second case is probably okay.  The first we can do nothing about.  
> > However, in either case we do need to make sure that memory allocations
> > do not require any writebacks.  This suggests that we need to call
> > memalloc_noio_save() from within usb_unbind_interface().
> 
> I agree with the problem, but I fail to see why this issue would be
> specific to USB. Shouldn't this be done in the device core layer?

Only for drivers that are on the block-device writeback path.  The 
device core doesn't know which drivers these are.

Alan Stern

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On Mi, 2019-05-22 at 10:56 -0400, Alan Stern wrote:
> On Wed, 22 May 2019, Oliver Neukum wrote:
> 
> > I agree with the problem, but I fail to see why this issue would be
> > specific to USB. Shouldn't this be done in the device core layer?
> 
> Only for drivers that are on the block-device writeback path.  The 
> device core doesn't know which drivers these are.

Neither does USB know. It is very hard to predict or even tell which
devices are block device drivers. I think we must assume that
any device may be affected.

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Alan Stern]

On Wed, 22 May 2019, Oliver Neukum wrote:

> On Mi, 2019-05-22 at 10:56 -0400, Alan Stern wrote:
> > On Wed, 22 May 2019, Oliver Neukum wrote:
> > 
> > > I agree with the problem, but I fail to see why this issue would be
> > > specific to USB. Shouldn't this be done in the device core layer?
> > 
> > Only for drivers that are on the block-device writeback path.  The 
> > device core doesn't know which drivers these are.
> 
> Neither does USB know. It is very hard to predict or even tell which
> devices are block device drivers. I think we must assume that
> any device may be affected.

All right.  Would you like to submit a patch?

Alan Stern

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

Am Donnerstag, den 23.05.2019, 10:01 -0400 schrieb Alan Stern:
> On Wed, 22 May 2019, Oliver Neukum wrote:
> 
> > On Mi, 2019-05-22 at 10:56 -0400, Alan Stern wrote:
> > > On Wed, 22 May 2019, Oliver Neukum wrote:
> > > 
> > > > I agree with the problem, but I fail to see why this issue would be
> > > > specific to USB. Shouldn't this be done in the device core layer?
> > > 
> > > Only for drivers that are on the block-device writeback path.  The 
> > > device core doesn't know which drivers these are.
> > 
> > Neither does USB know. It is very hard to predict or even tell which
> > devices are block device drivers. I think we must assume that
> > any device may be affected.
> 
> All right.  Would you like to submit a patch?

Do you like this one?

	Regards
		Oliver

[-- Attachment #2: 0001-base-force-NOIO-allocations-during-unplug.patch --]
[-- Type: text/x-patch, Size: 1522 bytes --]

From 0dc9c7dfe994fc9c28a63ba283e4442c237f6989 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Tue, 28 May 2019 11:43:02 +0200
Subject: [PATCH] base: force NOIO allocations during unplug

There is one overlooked situation under which a driver
must not do IO to allocate memory. You cannot do that
while disconnecting a device. A device being disconnected
is no longer functional in most cases, yet IO may fail
only when the handler runs.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/base/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index fd7511e04e62..a7f5f45bd761 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2229,6 +2229,7 @@ void device_del(struct device *dev)
 	struct device *parent = dev->parent;
 	struct kobject *glue_dir = NULL;
 	struct class_interface *class_intf;
+	unsigned int noio_flag;
 
 	/*
 	 * Hold the device lock and set the "dead" flag to guarantee that
@@ -2256,6 +2257,7 @@ void device_del(struct device *dev)
 		device_remove_sys_dev_entry(dev);
 		device_remove_file(dev, &dev_attr_dev);
 	}
+	noio_flag = memalloc_noio_save();
 	if (dev->class) {
 		device_remove_class_symlinks(dev);
 
@@ -2277,6 +2279,8 @@ void device_del(struct device *dev)
 	device_platform_notify(dev, KOBJ_REMOVE);
 	device_remove_properties(dev);
 	device_links_purge(dev);
+	memalloc_noio_restore(noio_flag);
+
 
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
-- 
2.16.4

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Alan Stern]

On Tue, 28 May 2019, Oliver Neukum wrote:

> Am Donnerstag, den 23.05.2019, 10:01 -0400 schrieb Alan Stern:
> > On Wed, 22 May 2019, Oliver Neukum wrote:
> > 
> > > On Mi, 2019-05-22 at 10:56 -0400, Alan Stern wrote:
> > > > On Wed, 22 May 2019, Oliver Neukum wrote:
> > > > 
> > > > > I agree with the problem, but I fail to see why this issue would be
> > > > > specific to USB. Shouldn't this be done in the device core layer?
> > > > 
> > > > Only for drivers that are on the block-device writeback path.  The 
> > > > device core doesn't know which drivers these are.
> > > 
> > > Neither does USB know. It is very hard to predict or even tell which
> > > devices are block device drivers. I think we must assume that
> > > any device may be affected.
> > 
> > All right.  Would you like to submit a patch?
> 
> Do you like this one?

Hmmm.  I might be inclined to move the start of the I/O-protected
region a little earlier.  For example, the first
blocking_notifier_call_chain() might result in some memory allocations.

The end is okay; once bus_remove_device() has returned the driver will 
be completely unbound, so there shouldn't be any pending I/O through 
the device.

Alan Stern

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Oliver Neukum]

On So, 2019-05-19 at 22:56 -0700, Christoph Hellwig wrote:
> Folks, you can't just pass arbitary GFP_ flags to dma allocation
> routines, beause very often they are not just wrappers around
> the page allocator.
> 
> So no, you can't just fine grained control the flags, but the
> existing code is just as buggy.
> 
> Please switch to use memalloc_noio_save() instead.

Thinking about this again, we have a problem. We introduced
memalloc_noio_save() in 3.10 . Hence the code should have been
correct in v4.14. Which means that either
6518202970c1 "(mm/cma: remove unsupported gfp_mask
parameter from cma_alloc()"
is buggy, or the original issue with a delay of 2 seconds
still exist.

Do we need to do something?

	Regards
		Oliver

Re: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation

[Christoph Hellwig]

On Thu, May 23, 2019 at 02:32:09PM +0200, Oliver Neukum wrote:
> > Please switch to use memalloc_noio_save() instead.
> 
> Thinking about this again, we have a problem. We introduced
> memalloc_noio_save() in 3.10 . Hence the code should have been
> correct in v4.14. Which means that either
> 6518202970c1 "(mm/cma: remove unsupported gfp_mask
> parameter from cma_alloc()"
> is buggy, or the original issue with a delay of 2 seconds
> still exist.
> 
> Do we need to do something?

cma_alloc calls into alloc_contig_range to do the actual allocation,
which then calls current_gfp_context() to pick up the adjustments
from memalloc_noio_save and friends.  So at least in current mainline
we should be fine.