Skip to content

Commit

Permalink
can: gs_usb: remove dma allocations
Browse files Browse the repository at this point in the history
DMA allocated buffers are a precious resource. If there is no need for
DMA allocations, then it might be worth to use non-dma allocated
buffers.

After testing the gs_usb driver with and without DMA allocation, there
does not seem to be a significant change in latency or CPU utilization
either way. Therefore, DMA allocation is not necessary and removed.

Internal buffers used within urbs were managed and freed manually.
These buffers are no longer needed to be managed by the driver. The
URB_FREE_BUFFER flag, allows for the buffers in question to be
automatically freed.

Co-developed-by: Rhett Aultman <rhett.aultman@samsara.com>
Signed-off-by: Rhett Aultman <rhett.aultman@samsara.com>
Signed-off-by: Vasanth Sadhasivan <vasanth.sadhasivan@samsara.com>
Link: https://lore.kernel.org/all/20220920154724.861093-2-rhett.aultman@samsara.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
  • Loading branch information
Vasanth Sadhasivan authored and Marc Kleine-Budde committed Sep 23, 2022
1 parent 906e0e6 commit 62f102c
Showing 1 changed file with 6 additions and 33 deletions.
39 changes: 6 additions & 33 deletions drivers/net/can/usb/gs_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,6 @@ struct gs_can {

struct usb_anchor tx_submitted;
atomic_t active_tx_urbs;
void *rxbuf[GS_MAX_RX_URBS];
dma_addr_t rxbuf_dma[GS_MAX_RX_URBS];
};

/* usb interface struct */
Expand Down Expand Up @@ -710,9 +708,6 @@ static void gs_usb_xmit_callback(struct urb *urb)

if (urb->status)
netdev_info(netdev, "usb xmit fail %u\n", txc->echo_id);

usb_free_coherent(urb->dev, urb->transfer_buffer_length,
urb->transfer_buffer, urb->transfer_dma);
}

static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
Expand Down Expand Up @@ -741,8 +736,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
if (!urb)
goto nomem_urb;

hf = usb_alloc_coherent(dev->udev, dev->hf_size_tx, GFP_ATOMIC,
&urb->transfer_dma);
hf = kmalloc(dev->hf_size_tx, GFP_ATOMIC);
if (!hf) {
netdev_err(netdev, "No memory left for USB buffer\n");
goto nomem_hf;
Expand Down Expand Up @@ -786,7 +780,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
hf, dev->hf_size_tx,
gs_usb_xmit_callback, txc);

urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
urb->transfer_flags |= URB_FREE_BUFFER;
usb_anchor_urb(urb, &dev->tx_submitted);

can_put_echo_skb(skb, netdev, idx, 0);
Expand All @@ -801,8 +795,6 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
gs_free_tx_context(txc);

usb_unanchor_urb(urb);
usb_free_coherent(dev->udev, urb->transfer_buffer_length,
urb->transfer_buffer, urb->transfer_dma);

if (rc == -ENODEV) {
netif_device_detach(netdev);
Expand All @@ -822,8 +814,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
return NETDEV_TX_OK;

badidx:
usb_free_coherent(dev->udev, urb->transfer_buffer_length,
urb->transfer_buffer, urb->transfer_dma);
kfree(hf);
nomem_hf:
usb_free_urb(urb);

Expand Down Expand Up @@ -869,27 +860,22 @@ static int gs_can_open(struct net_device *netdev)
for (i = 0; i < GS_MAX_RX_URBS; i++) {
struct urb *urb;
u8 *buf;
dma_addr_t buf_dma;

/* alloc rx urb */
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb)
return -ENOMEM;

/* alloc rx buffer */
buf = usb_alloc_coherent(dev->udev,
dev->parent->hf_size_rx,
GFP_KERNEL,
&buf_dma);
buf = kmalloc(dev->parent->hf_size_rx,
GFP_KERNEL);
if (!buf) {
netdev_err(netdev,
"No memory left for USB buffer\n");
usb_free_urb(urb);
return -ENOMEM;
}

urb->transfer_dma = buf_dma;

/* fill, anchor, and submit rx urb */
usb_fill_bulk_urb(urb,
dev->udev,
Expand All @@ -898,7 +884,7 @@ static int gs_can_open(struct net_device *netdev)
buf,
dev->parent->hf_size_rx,
gs_usb_receive_bulk_callback, parent);
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
urb->transfer_flags |= URB_FREE_BUFFER;

usb_anchor_urb(urb, &parent->rx_submitted);

Expand All @@ -911,17 +897,10 @@ static int gs_can_open(struct net_device *netdev)
"usb_submit failed (err=%d)\n", rc);

usb_unanchor_urb(urb);
usb_free_coherent(dev->udev,
sizeof(struct gs_host_frame),
buf,
buf_dma);
usb_free_urb(urb);
break;
}

dev->rxbuf[i] = buf;
dev->rxbuf_dma[i] = buf_dma;

/* Drop reference,
* USB core will take care of freeing it
*/
Expand Down Expand Up @@ -980,7 +959,6 @@ static int gs_can_close(struct net_device *netdev)
int rc;
struct gs_can *dev = netdev_priv(netdev);
struct gs_usb *parent = dev->parent;
unsigned int i;

netif_stop_queue(netdev);

Expand All @@ -992,11 +970,6 @@ static int gs_can_close(struct net_device *netdev)
parent->active_channels--;
if (!parent->active_channels) {
usb_kill_anchored_urbs(&parent->rx_submitted);
for (i = 0; i < GS_MAX_RX_URBS; i++)
usb_free_coherent(dev->udev,
sizeof(struct gs_host_frame),
dev->rxbuf[i],
dev->rxbuf_dma[i]);
}

/* Stop sending URBs */
Expand Down

0 comments on commit 62f102c

Please sign in to comment.