Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
From:   Jaewon Kim <jaewon31.kim@gmail.com>
Date:   Sat, 18 May 2019 01:02:28 +0900
Message-ID: <CAJrd-UuMRdWHky4gkmiR0QYozfXW0O35Ohv6mJPFx2TLa8hRKg@mail.gmail.com>
Subject: [RFC PATCH] usb: host: xhci: allow __GFP_FS in dma allocation
To:     gregkh@linuxfoundation.org, m.szyprowski@samsung.com,
        linux-mm@kvack.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Jaewon Kim <jaewon31.kim@samsung.com>, ytk.lee@samsung.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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