* [PATCH net-next v2 0/3] virtio: style cleanups
@ 2016-07-26 16:37 Stephen Hemminger
2016-07-26 16:37 ` [PATCH net-next v2 1/3] virtio: whitespace cleanups Stephen Hemminger
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Stephen Hemminger @ 2016-07-26 16:37 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
Minor things found while doing some inspection code review.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v2 1/3] virtio: whitespace cleanups 2016-07-26 16:37 [PATCH net-next v2 0/3] virtio: style cleanups Stephen Hemminger @ 2016-07-26 16:37 ` Stephen Hemminger 2016-07-26 16:58 ` Michael S. Tsirkin 2016-07-26 16:37 ` [PATCH net-next v2 2/3] virtio: cleanup sizeof usage Stephen Hemminger ` (2 subsequent siblings) 3 siblings, 1 reply; 6+ messages in thread From: Stephen Hemminger @ 2016-07-26 16:37 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: netdev, Stephen Hemminger [-- Attachment #1: virtio-style.patch --] [-- Type: text/plain, Size: 6531 bytes --] Adjust whitespace to match current favored style. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- a/drivers/net/virtio_net.c 2016-07-20 08:12:24.035656119 -0700 +++ b/drivers/net/virtio_net.c 2016-07-26 09:11:06.988788668 -0700 @@ -201,7 +201,9 @@ static void give_pages(struct receive_qu struct page *end; /* Find end of list, sew whole thing into vi->rq.pages. */ - for (end = page; end->private; end = (struct page *)end->private); + for (end = page; end->private; end = (struct page *)end->private) + continue; + end->private = (unsigned long)rq->pages; rq->pages = page; } @@ -233,13 +235,13 @@ static void skb_xmit_done(struct virtque static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) { unsigned int truesize = mrg_ctx & (MERGEABLE_BUFFER_ALIGN - 1); + return (truesize + 1) * MERGEABLE_BUFFER_ALIGN; } static void *mergeable_ctx_to_buf_address(unsigned long mrg_ctx) { return (void *)(mrg_ctx & -MERGEABLE_BUFFER_ALIGN); - } static unsigned long mergeable_buf_to_ctx(void *buf, unsigned int truesize) @@ -310,6 +312,7 @@ static struct sk_buff *page_to_skb(struc BUG_ON(offset >= PAGE_SIZE); while (len) { unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len); + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset, frag_size, truesize); len -= frag_size; @@ -323,9 +326,10 @@ static struct sk_buff *page_to_skb(struc return skb; } -static struct sk_buff *receive_small(struct virtnet_info *vi, void *buf, unsigned int len) +static struct sk_buff *receive_small(struct virtnet_info *vi, + void *buf, unsigned int len) { - struct sk_buff * skb = buf; + struct sk_buff *skb = buf; len -= vi->hdr_len; skb_trim(skb, len); @@ -455,6 +459,7 @@ static void receive_buf(struct virtnet_i if (vi->mergeable_rx_bufs) { unsigned long ctx = (unsigned long)buf; void *base = mergeable_ctx_to_buf_address(ctx); + put_page(virt_to_head_page(base)); } else if (vi->big_packets) { give_pages(rq, buf); @@ -699,7 +704,7 @@ static void refill_work(struct work_stru * we will *never* try to fill again. */ if (still_empty) - schedule_delayed_work(&vi->refill, HZ/2); + schedule_delayed_work(&vi->refill, HZ / 2); } } @@ -901,12 +906,12 @@ static netdev_tx_t start_xmit(struct sk_ * Since most packets only take 1 or 2 ring slots, stopping the queue * early means 16 slots are typically wasted. */ - if (sq->vq->num_free < 2+MAX_SKB_FRAGS) { + if (sq->vq->num_free < 2 + MAX_SKB_FRAGS) { netif_stop_subqueue(dev, qnum); if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) { /* More just got used, free them then recheck. */ free_old_xmit_skbs(sq); - if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) { + if (sq->vq->num_free >= 2 + MAX_SKB_FRAGS) { netif_start_subqueue(dev, qnum); virtqueue_disable_cb(sq->vq); } @@ -1238,11 +1243,11 @@ static void virtnet_set_affinity(struct } static int virtnet_cpu_callback(struct notifier_block *nfb, - unsigned long action, void *hcpu) + unsigned long action, void *hcpu) { struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb); - switch(action & ~CPU_TASKS_FROZEN) { + switch (action & ~CPU_TASKS_FROZEN) { case CPU_ONLINE: case CPU_DOWN_FAILED: case CPU_DEAD: @@ -1259,7 +1264,7 @@ static int virtnet_cpu_callback(struct n } static void virtnet_get_ringparam(struct net_device *dev, - struct ethtool_ringparam *ring) + struct ethtool_ringparam *ring) { struct virtnet_info *vi = netdev_priv(dev); @@ -1269,7 +1274,6 @@ static void virtnet_get_ringparam(struct ring->tx_pending = ring->tx_max_pending; } - static void virtnet_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info) { @@ -1279,7 +1283,6 @@ static void virtnet_get_drvinfo(struct n strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver)); strlcpy(info->version, VIRTNET_DRIVER_VERSION, sizeof(info->version)); strlcpy(info->bus_info, virtio_bus_name(vdev), sizeof(info->bus_info)); - } /* TODO: Eliminate OOO packets during switching */ @@ -1402,9 +1405,9 @@ static int virtnet_change_mtu(struct net } static const struct net_device_ops virtnet_netdev = { - .ndo_open = virtnet_open, - .ndo_stop = virtnet_close, - .ndo_start_xmit = start_xmit, + .ndo_open = virtnet_open, + .ndo_stop = virtnet_close, + .ndo_start_xmit = start_xmit, .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = virtnet_set_mac_address, .ndo_set_rx_mode = virtnet_set_rx_mode, @@ -1485,6 +1488,7 @@ static void free_receive_bufs(struct vir static void free_receive_page_frags(struct virtnet_info *vi) { int i; + for (i = 0; i < vi->max_queue_pairs; i++) if (vi->rq[i].alloc_frag.page) put_page(vi->rq[i].alloc_frag.page); @@ -1497,6 +1501,7 @@ static void free_unused_bufs(struct virt for (i = 0; i < vi->max_queue_pairs; i++) { struct virtqueue *vq = vi->sq[i].vq; + while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) dev_kfree_skb(buf); } @@ -1508,6 +1513,7 @@ static void free_unused_bufs(struct virt if (vi->mergeable_rx_bufs) { unsigned long ctx = (unsigned long)buf; void *base = mergeable_ctx_to_buf_address(ctx); + put_page(virt_to_head_page(base)); } else if (vi->big_packets) { give_pages(&vi->rq[i], buf); @@ -1660,7 +1666,8 @@ err: #ifdef CONFIG_SYSFS static ssize_t mergeable_rx_buffer_size_show(struct netdev_rx_queue *queue, - struct rx_queue_attribute *attribute, char *buf) + struct rx_queue_attribute *attribute, + char *buf) { struct virtnet_info *vi = netdev_priv(queue->dev); unsigned int queue_index = get_netdev_rx_queue_index(queue); @@ -1784,7 +1791,8 @@ static int virtnet_probe(struct virtio_d dev->features |= NETIF_F_GSO_ROBUST; if (gso) - dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO); + dev->features |= dev->hw_features + & (NETIF_F_ALL_TSO | NETIF_F_UFO); /* (!csum && gso) case will be fixed by register_netdev() */ } if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM)) @@ -1811,8 +1819,9 @@ static int virtnet_probe(struct virtio_d goto free; for_each_possible_cpu(i) { - struct virtnet_stats *virtnet_stats; - virtnet_stats = per_cpu_ptr(vi->stats, i); + struct virtnet_stats *virtnet_stats + = per_cpu_ptr(vi->stats, i); + u64_stats_init(&virtnet_stats->tx_syncp); u64_stats_init(&virtnet_stats->rx_syncp); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 1/3] virtio: whitespace cleanups 2016-07-26 16:37 ` [PATCH net-next v2 1/3] virtio: whitespace cleanups Stephen Hemminger @ 2016-07-26 16:58 ` Michael S. Tsirkin 0 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2016-07-26 16:58 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev On Tue, Jul 26, 2016 at 09:37:13AM -0700, Stephen Hemminger wrote: > Adjust whitespace to match current favored style. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > --- a/drivers/net/virtio_net.c 2016-07-20 08:12:24.035656119 -0700 > +++ b/drivers/net/virtio_net.c 2016-07-26 09:11:06.988788668 -0700 > @@ -201,7 +201,9 @@ static void give_pages(struct receive_qu > struct page *end; > > /* Find end of list, sew whole thing into vi->rq.pages. */ > - for (end = page; end->private; end = (struct page *)end->private); > + for (end = page; end->private; end = (struct page *)end->private) > + continue; > + > end->private = (unsigned long)rq->pages; > rq->pages = page; > } > @@ -233,13 +235,13 @@ static void skb_xmit_done(struct virtque > static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) > { > unsigned int truesize = mrg_ctx & (MERGEABLE_BUFFER_ALIGN - 1); > + > return (truesize + 1) * MERGEABLE_BUFFER_ALIGN; > } > > static void *mergeable_ctx_to_buf_address(unsigned long mrg_ctx) > { > return (void *)(mrg_ctx & -MERGEABLE_BUFFER_ALIGN); > - > } > > static unsigned long mergeable_buf_to_ctx(void *buf, unsigned int truesize) > @@ -310,6 +312,7 @@ static struct sk_buff *page_to_skb(struc > BUG_ON(offset >= PAGE_SIZE); > while (len) { > unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len); > + > skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset, > frag_size, truesize); > len -= frag_size; > @@ -323,9 +326,10 @@ static struct sk_buff *page_to_skb(struc > return skb; > } > > -static struct sk_buff *receive_small(struct virtnet_info *vi, void *buf, unsigned int len) > +static struct sk_buff *receive_small(struct virtnet_info *vi, > + void *buf, unsigned int len) > { > - struct sk_buff * skb = buf; > + struct sk_buff *skb = buf; > > len -= vi->hdr_len; > skb_trim(skb, len); > @@ -455,6 +459,7 @@ static void receive_buf(struct virtnet_i > if (vi->mergeable_rx_bufs) { > unsigned long ctx = (unsigned long)buf; > void *base = mergeable_ctx_to_buf_address(ctx); > + > put_page(virt_to_head_page(base)); > } else if (vi->big_packets) { > give_pages(rq, buf); > @@ -699,7 +704,7 @@ static void refill_work(struct work_stru > * we will *never* try to fill again. > */ > if (still_empty) > - schedule_delayed_work(&vi->refill, HZ/2); > + schedule_delayed_work(&vi->refill, HZ / 2); > } > } > > @@ -901,12 +906,12 @@ static netdev_tx_t start_xmit(struct sk_ > * Since most packets only take 1 or 2 ring slots, stopping the queue > * early means 16 slots are typically wasted. > */ > - if (sq->vq->num_free < 2+MAX_SKB_FRAGS) { > + if (sq->vq->num_free < 2 + MAX_SKB_FRAGS) { > netif_stop_subqueue(dev, qnum); > if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) { > /* More just got used, free them then recheck. */ > free_old_xmit_skbs(sq); > - if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) { > + if (sq->vq->num_free >= 2 + MAX_SKB_FRAGS) { > netif_start_subqueue(dev, qnum); > virtqueue_disable_cb(sq->vq); > } > @@ -1238,11 +1243,11 @@ static void virtnet_set_affinity(struct > } > > static int virtnet_cpu_callback(struct notifier_block *nfb, > - unsigned long action, void *hcpu) > + unsigned long action, void *hcpu) > { > struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb); > > - switch(action & ~CPU_TASKS_FROZEN) { > + switch (action & ~CPU_TASKS_FROZEN) { > case CPU_ONLINE: > case CPU_DOWN_FAILED: > case CPU_DEAD: > @@ -1259,7 +1264,7 @@ static int virtnet_cpu_callback(struct n > } > > static void virtnet_get_ringparam(struct net_device *dev, > - struct ethtool_ringparam *ring) > + struct ethtool_ringparam *ring) > { > struct virtnet_info *vi = netdev_priv(dev); > > @@ -1269,7 +1274,6 @@ static void virtnet_get_ringparam(struct > ring->tx_pending = ring->tx_max_pending; > } > > - > static void virtnet_get_drvinfo(struct net_device *dev, > struct ethtool_drvinfo *info) > { > @@ -1279,7 +1283,6 @@ static void virtnet_get_drvinfo(struct n > strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver)); > strlcpy(info->version, VIRTNET_DRIVER_VERSION, sizeof(info->version)); > strlcpy(info->bus_info, virtio_bus_name(vdev), sizeof(info->bus_info)); > - > } > > /* TODO: Eliminate OOO packets during switching */ > @@ -1402,9 +1405,9 @@ static int virtnet_change_mtu(struct net > } > > static const struct net_device_ops virtnet_netdev = { > - .ndo_open = virtnet_open, > - .ndo_stop = virtnet_close, > - .ndo_start_xmit = start_xmit, > + .ndo_open = virtnet_open, > + .ndo_stop = virtnet_close, > + .ndo_start_xmit = start_xmit, > .ndo_validate_addr = eth_validate_addr, > .ndo_set_mac_address = virtnet_set_mac_address, > .ndo_set_rx_mode = virtnet_set_rx_mode, > @@ -1485,6 +1488,7 @@ static void free_receive_bufs(struct vir > static void free_receive_page_frags(struct virtnet_info *vi) > { > int i; > + > for (i = 0; i < vi->max_queue_pairs; i++) > if (vi->rq[i].alloc_frag.page) > put_page(vi->rq[i].alloc_frag.page); > @@ -1497,6 +1501,7 @@ static void free_unused_bufs(struct virt > > for (i = 0; i < vi->max_queue_pairs; i++) { > struct virtqueue *vq = vi->sq[i].vq; > + > while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) > dev_kfree_skb(buf); > } > @@ -1508,6 +1513,7 @@ static void free_unused_bufs(struct virt > if (vi->mergeable_rx_bufs) { > unsigned long ctx = (unsigned long)buf; > void *base = mergeable_ctx_to_buf_address(ctx); > + > put_page(virt_to_head_page(base)); > } else if (vi->big_packets) { > give_pages(&vi->rq[i], buf); > @@ -1660,7 +1666,8 @@ err: > > #ifdef CONFIG_SYSFS > static ssize_t mergeable_rx_buffer_size_show(struct netdev_rx_queue *queue, > - struct rx_queue_attribute *attribute, char *buf) > + struct rx_queue_attribute *attribute, > + char *buf) > { > struct virtnet_info *vi = netdev_priv(queue->dev); > unsigned int queue_index = get_netdev_rx_queue_index(queue); > @@ -1784,7 +1791,8 @@ static int virtnet_probe(struct virtio_d > dev->features |= NETIF_F_GSO_ROBUST; > > if (gso) > - dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO); > + dev->features |= dev->hw_features > + & (NETIF_F_ALL_TSO | NETIF_F_UFO); I'm sorry, I don't like continuations to start with an operator on the next line. I prefer & on the previous line instead. Is this one a preferred style now? Any pointers? > /* (!csum && gso) case will be fixed by register_netdev() */ > } > if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM)) > @@ -1811,8 +1819,9 @@ static int virtnet_probe(struct virtio_d > goto free; > > for_each_possible_cpu(i) { > - struct virtnet_stats *virtnet_stats; > - virtnet_stats = per_cpu_ptr(vi->stats, i); > + struct virtnet_stats *virtnet_stats > + = per_cpu_ptr(vi->stats, i); > + Same here for =. > u64_stats_init(&virtnet_stats->tx_syncp); > u64_stats_init(&virtnet_stats->rx_syncp); > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/3] virtio: cleanup sizeof usage 2016-07-26 16:37 [PATCH net-next v2 0/3] virtio: style cleanups Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 1/3] virtio: whitespace cleanups Stephen Hemminger @ 2016-07-26 16:37 ` Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 3/3] virtio: use kcalloc and kmalloc_array Stephen Hemminger 2016-07-26 18:11 ` [PATCH net-next v2 0/3] virtio: style cleanups Michael S. Tsirkin 3 siblings, 0 replies; 6+ messages in thread From: Stephen Hemminger @ 2016-07-26 16:37 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: netdev, Stephen Hemminger [-- Attachment #1: virtio-sizeof.patch --] [-- Type: text/plain, Size: 551 bytes --] Although sizeof is an operator in C. Preferred usage is to use it as a function. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- a/drivers/net/virtio_net.c 2016-07-26 09:13:16.173384238 -0700 +++ b/drivers/net/virtio_net.c 2016-07-26 09:13:16.169384220 -0700 @@ -272,7 +272,7 @@ static struct sk_buff *page_to_skb(struc hdr_len = vi->hdr_len; if (vi->mergeable_rx_bufs) - hdr_padded_len = sizeof *hdr; + hdr_padded_len = sizeof(struct virtio_net_hdr_mrg_rxbuf); else hdr_padded_len = sizeof(struct padded_vnet_hdr); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 3/3] virtio: use kcalloc and kmalloc_array 2016-07-26 16:37 [PATCH net-next v2 0/3] virtio: style cleanups Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 1/3] virtio: whitespace cleanups Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 2/3] virtio: cleanup sizeof usage Stephen Hemminger @ 2016-07-26 16:37 ` Stephen Hemminger 2016-07-26 18:11 ` [PATCH net-next v2 0/3] virtio: style cleanups Michael S. Tsirkin 3 siblings, 0 replies; 6+ messages in thread From: Stephen Hemminger @ 2016-07-26 16:37 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: netdev, Stephen Hemminger [-- Attachment #1: virtio-kcalloc.patch --] [-- Type: text/plain, Size: 1323 bytes --] Preferred style is to use kcalloc and kmalloc_array. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- a/drivers/net/virtio_net.c 2016-07-26 09:28:21.401557546 -0700 +++ b/drivers/net/virtio_net.c 2016-07-26 09:36:05.107695337 -0700 @@ -1551,13 +1551,13 @@ static int virtnet_find_vqs(struct virtn virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ); /* Allocate space for find_vqs parameters */ - vqs = kzalloc(total_vqs * sizeof(*vqs), GFP_KERNEL); + vqs = kcalloc(total_vqs, sizeof(*vqs), GFP_KERNEL); if (!vqs) goto err_vq; - callbacks = kmalloc(total_vqs * sizeof(*callbacks), GFP_KERNEL); + callbacks = kmalloc_array(total_vqs, sizeof(*callbacks), GFP_KERNEL); if (!callbacks) goto err_callback; - names = kmalloc(total_vqs * sizeof(*names), GFP_KERNEL); + names = kmalloc_array(total_vqs, sizeof(*names), GFP_KERNEL); if (!names) goto err_names; @@ -1613,10 +1613,10 @@ static int virtnet_alloc_queues(struct v { int i; - vi->sq = kzalloc(sizeof(*vi->sq) * vi->max_queue_pairs, GFP_KERNEL); + vi->sq = kcalloc(vi->max_queue_pairs, sizeof(*vi->sq), GFP_KERNEL); if (!vi->sq) goto err_sq; - vi->rq = kzalloc(sizeof(*vi->rq) * vi->max_queue_pairs, GFP_KERNEL); + vi->rq = kcalloc(vi->max_queue_pairs, sizeof(*vi->rq), GFP_KERNEL); if (!vi->rq) goto err_rq; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 0/3] virtio: style cleanups 2016-07-26 16:37 [PATCH net-next v2 0/3] virtio: style cleanups Stephen Hemminger ` (2 preceding siblings ...) 2016-07-26 16:37 ` [PATCH net-next v2 3/3] virtio: use kcalloc and kmalloc_array Stephen Hemminger @ 2016-07-26 18:11 ` Michael S. Tsirkin 3 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2016-07-26 18:11 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev On Tue, Jul 26, 2016 at 09:37:12AM -0700, Stephen Hemminger wrote: > Minor things found while doing some inspection code review. Acked-by: Michael S. Tsirkin <mst@redhat.com> I guess I'll just queue it up for this release. -- MST ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-26 18:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-26 16:37 [PATCH net-next v2 0/3] virtio: style cleanups Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 1/3] virtio: whitespace cleanups Stephen Hemminger 2016-07-26 16:58 ` Michael S. Tsirkin 2016-07-26 16:37 ` [PATCH net-next v2 2/3] virtio: cleanup sizeof usage Stephen Hemminger 2016-07-26 16:37 ` [PATCH net-next v2 3/3] virtio: use kcalloc and kmalloc_array Stephen Hemminger 2016-07-26 18:11 ` [PATCH net-next v2 0/3] virtio: style cleanups Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).