[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1401126135.17377.134.camel@deadeye.wl.decadent.org.uk>
Date: Mon, 26 May 2014 18:42:15 +0100
From: Ben Hutchings <ben@...adent.org.uk>
To: Govindarajulu Varadarajan <_govind@....com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, benve@...co.com,
neepatel@...co.com, ssujith@...co.com
Subject: Re: [PATCH v2 net-next 1/2] enic: Add support for adaptive
interrupt coalescing
On Tue, 2014-05-20 at 03:14 +0530, Govindarajulu Varadarajan wrote:
[...]
> @@ -191,17 +207,31 @@ static int enic_set_coalesce(struct net_device *netdev,
> struct enic *enic = netdev_priv(netdev);
> u32 tx_coalesce_usecs;
> u32 rx_coalesce_usecs;
> + u32 rx_coalesce_usecs_low;
> + u32 rx_coalesce_usecs_high;
> + u32 coalesce_usecs_max;
> unsigned int i, intr;
> + struct enic_rx_coal *rxcoal = &enic->rx_coalesce_setting;
>
> + coalesce_usecs_max = vnic_dev_get_intr_coal_timer_max(enic->vdev);
> tx_coalesce_usecs = min_t(u32, ecmd->tx_coalesce_usecs,
> - vnic_dev_get_intr_coal_timer_max(enic->vdev));
> + coalesce_usecs_max);
> rx_coalesce_usecs = min_t(u32, ecmd->rx_coalesce_usecs,
> - vnic_dev_get_intr_coal_timer_max(enic->vdev));
> + coalesce_usecs_max);
> +
> + rx_coalesce_usecs_low = min_t(u32, ecmd->rx_coalesce_usecs_low,
> + coalesce_usecs_max);
> + rx_coalesce_usecs_high = min_t(u32, ecmd->rx_coalesce_usecs_high,
> + coalesce_usecs_max);
>
> switch (vnic_dev_get_intr_mode(enic->vdev)) {
> case VNIC_DEV_INTR_MODE_INTX:
> if (tx_coalesce_usecs != rx_coalesce_usecs)
> return -EINVAL;
> + if (ecmd->use_adaptive_rx_coalesce ||
> + ecmd->rx_coalesce_usecs_low ||
> + ecmd->rx_coalesce_usecs_high)
> + return -EOPNOTSUPP;
-EINVAL, please. The operation (set coalescing parameters) is
supported, just not these particular arguments.
> intr = enic_legacy_io_intr();
> vnic_intr_coalescing_timer_set(&enic->intr[intr],
> @@ -210,6 +240,10 @@ static int enic_set_coalesce(struct net_device *netdev,
> case VNIC_DEV_INTR_MODE_MSI:
> if (tx_coalesce_usecs != rx_coalesce_usecs)
> return -EINVAL;
> + if (ecmd->use_adaptive_rx_coalesce ||
> + ecmd->rx_coalesce_usecs_low ||
> + ecmd->rx_coalesce_usecs_high)
> + return -EOPNOTSUPP;
-EINVAL again.
> vnic_intr_coalescing_timer_set(&enic->intr[0],
> tx_coalesce_usecs);
> @@ -221,12 +255,27 @@ static int enic_set_coalesce(struct net_device *netdev,
> tx_coalesce_usecs);
> }
>
> - for (i = 0; i < enic->rq_count; i++) {
> - intr = enic_msix_rq_intr(enic, i);
> - vnic_intr_coalescing_timer_set(&enic->intr[intr],
> - rx_coalesce_usecs);
> + if (rxcoal->use_adaptive_rx_coalesce) {
> + if (!ecmd->use_adaptive_rx_coalesce) {
> + rxcoal->use_adaptive_rx_coalesce = 0;
> + enic_intr_coal_set_rx(enic, rx_coalesce_usecs);
> + }
> + } else {
> + if (ecmd->use_adaptive_rx_coalesce)
> + rxcoal->use_adaptive_rx_coalesce = 1;
> + else
> + enic_intr_coal_set_rx(enic, rx_coalesce_usecs);
> }
This seems to be an overly complicated way of writing:
rxcoal->use_adaptive_rx_coalesce = !!ecmd->use_adaptive_rx_coalesce;
if (!rxcoal->use_adaptive_rx_coalesce)
enic_intr_coal_set_rx(enic, rx_coalesce_usecs);
> + if (ecmd->rx_coalesce_usecs_high) {
> + if (rx_coalesce_usecs_high <
> + (rx_coalesce_usecs_low + ENIC_AIC_LARGE_PKT_DIFF))
> + return -EINVAL;
[...]
It's a bit late to detect invalid arguments when you've already started
applying changes.
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)
Powered by blists - more mailing lists