2019-06-18 14:29:29

by Suwan Kim

Subject: [PATCH] usbip: Implement map_urb_for_dma function for vhci to skip dma mapping

vhci doesn’t do dma for remote device. Actually, the real dma
operation is done by network card driver. So, vhci doesn’t use and
need dma address of transfer buffer of urb.

But hcd provides dma mapping function by defualt in usb_hcd_submit_urb()
and it causes unnecessary dma mapping which will be done again at
NIC driver and it wastes CPU cycles. So, implement map_urb_for_dma
function for vhci in order to skip the dma mapping procedure.

Signed-off-by: Suwan Kim <[email protected]>
---
drivers/usb/usbip/vhci_hcd.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 667d9c0ec905..9df4d9e36788 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1287,6 +1287,13 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
return 0;
}

+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ dev_dbg(hcd->self.controller, "vhci does not map urb for dma\n");
+ return 0;
+}
+
static const struct hc_driver vhci_hc_driver = {
.description = driver_name,
.product_desc = driver_desc,
@@ -1302,6 +1309,7 @@ static const struct hc_driver vhci_hc_driver = {
.urb_dequeue = vhci_urb_dequeue,

.get_frame_number = vhci_get_frame_number,
+ .map_urb_for_dma = vhci_map_urb_for_dma,

.hub_status_data = vhci_hub_status,
.hub_control = vhci_hub_control,
--
2.20.1

2019-06-18 15:31:19

by Alan Stern

Subject: Re: [PATCH] usbip: Implement map_urb_for_dma function for vhci to skip dma mapping

On Tue, 18 Jun 2019, Suwan Kim wrote:

> vhci doesn’t do dma for remote device. Actually, the real dma
> operation is done by network card driver. So, vhci doesn’t use and
> need dma address of transfer buffer of urb.
>
> But hcd provides dma mapping function by defualt in usb_hcd_submit_urb()
> and it causes unnecessary dma mapping which will be done again at
> NIC driver and it wastes CPU cycles. So, implement map_urb_for_dma
> function for vhci in order to skip the dma mapping procedure.
>
> Signed-off-by: Suwan Kim <[email protected]>
> ---
> drivers/usb/usbip/vhci_hcd.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 667d9c0ec905..9df4d9e36788 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1287,6 +1287,13 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
> return 0;
> }
>
> +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> + gfp_t mem_flags)
> +{
> + dev_dbg(hcd->self.controller, "vhci does not map urb for dma\n");
> + return 0;
> +}
> +
> static const struct hc_driver vhci_hc_driver = {
> .description = driver_name,
> .product_desc = driver_desc,
> @@ -1302,6 +1309,7 @@ static const struct hc_driver vhci_hc_driver = {
> .urb_dequeue = vhci_urb_dequeue,
>
> .get_frame_number = vhci_get_frame_number,
> + .map_urb_for_dma = vhci_map_urb_for_dma,
>
> .hub_status_data = vhci_hub_status,
> .hub_control = vhci_hub_control,

If the goal is to avoid wasting CPU cycles, you probably should have a
vhci_unmap_urb_for_dma routine as well.

Alan Stern

2019-06-19 08:18:12

by Suwan Kim

Subject: Re: [PATCH] usbip: Implement map_urb_for_dma function for vhci to skip dma mapping

On Tue, Jun 18, 2019 at 11:30:34AM -0400, Alan Stern wrote:
> On Tue, 18 Jun 2019, Suwan Kim wrote:
>
> > vhci doesn’t do dma for remote device. Actually, the real dma
> > operation is done by network card driver. So, vhci doesn’t use and
> > need dma address of transfer buffer of urb.
> >
> > But hcd provides dma mapping function by defualt in usb_hcd_submit_urb()
> > and it causes unnecessary dma mapping which will be done again at
> > NIC driver and it wastes CPU cycles. So, implement map_urb_for_dma
> > function for vhci in order to skip the dma mapping procedure.
> >
> > Signed-off-by: Suwan Kim <[email protected]>
> > ---
> > drivers/usb/usbip/vhci_hcd.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> > index 667d9c0ec905..9df4d9e36788 100644
> > --- a/drivers/usb/usbip/vhci_hcd.c
> > +++ b/drivers/usb/usbip/vhci_hcd.c
> > @@ -1287,6 +1287,13 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
> > return 0;
> > }
> >
> > +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> > + gfp_t mem_flags)
> > +{
> > + dev_dbg(hcd->self.controller, "vhci does not map urb for dma\n");
> > + return 0;
> > +}
> > +
> > static const struct hc_driver vhci_hc_driver = {
> > .description = driver_name,
> > .product_desc = driver_desc,
> > @@ -1302,6 +1309,7 @@ static const struct hc_driver vhci_hc_driver = {
> > .urb_dequeue = vhci_urb_dequeue,
> >
> > .get_frame_number = vhci_get_frame_number,
> > + .map_urb_for_dma = vhci_map_urb_for_dma,
> >
> > .hub_status_data = vhci_hub_status,
> > .hub_control = vhci_hub_control,
>
> If the goal is to avoid wasting CPU cycles, you probably should have a
> vhci_unmap_urb_for_dma routine as well.

I missed that. Thank you for pointing it out.
I will send v2 including unmap function.

Regards

Suwan Kim