Re: [PATCH net-next 4/8] enic: alloc/free rx_cpu_rmap
From: Govindarajulu Varadarajan <hidden>
Date: 2014-06-10 13:53:10
On Mon, 9 Jun 2014, Sergei Shtylyov wrote:
quoted hunk ↗ jump to hunk
@@ -1192,6 +1195,33 @@ static void enic_calc_int_moderation(struct enicquoted
*enic, struct vnic_rq *rq) pkt_size_counter->small_pkt_bytes_cnt = 0; } +#ifdef CONFIG_RFS_ACCEL +static void enic_free_rx_cpu_rmap(struct enic *enic) +{ + free_irq_cpu_rmap(enic->netdev->rx_cpu_rmap); + enic->netdev->rx_cpu_rmap = NULL; +} + +static inline void enic_set_rx_cpu_rmap(struct enic *enic)No need to use *inline* in a .c file, the compiler should figure it out.
Yes, I agree.
quoted
+{ + int i, res; + + if (vnic_dev_get_intr_mode(enic->vdev) == VNIC_DEV_INTR_MODE_MSIX) { + enic->netdev->rx_cpu_rmap = alloc_irq_cpu_rmap(enic->rq_count); + if (unlikely(!enic->netdev->rx_cpu_rmap)) + return; + for (i = 0; i < enic->rq_count; i++) { + res = irq_cpu_rmap_add(enic->netdev->rx_cpu_rmap, + enic->msix_entry[i].vector); + if (unlikely(res)) { + enic_free_rx_cpu_rmap(enic); + return; + } + } + } +}It's better to do the following here: #else static void enic_free_rx_cpu_rmap(struct enic *enic) { } static void enic_set_rx_cpu_rmap(struct enic *enic) { }
How about
static void enic_free_rx_cpu_rmap(struct enic *enic)
{
#ifdef CONFIG_RFS_ACCEL
...
...
#endif
}
I prefer this over yours because, if I use yours tools like cscope finds two
definitions of function enic_free_rx_cpu_rmap. Which makes code walk through
little bit difficult.
Thanks
Govind
quoted
+#endif + static int enic_poll_msix(struct napi_struct *napi, int budget) { struct net_device *netdev = napi->dev;@@ -1267,6 +1297,9 @@ static void enic_free_intr(struct enic *enic) struct net_device *netdev = enic->netdev; unsigned int i; +#ifdef CONFIG_RFS_ACCEL + enic_free_rx_cpu_rmap(enic); +#endif... so that you can avoid #ifdef's at the call sites. WBR, Sergei