netdev - Re: [PATCH] Improved network performance by balancing Rx against other work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 23 Mar 2010 00:27:55 +1100
From:	Nick Piggin <npiggin@...e.de>
To:	Peter Chubb <peter.chubb@...ta.com.au>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] Improved network performance by balancing Rx against
 other work

On Wed, Mar 17, 2010 at 01:55:58PM +1100, Peter Chubb wrote:
> +static int e1000_intr_thread(void *data)
> +{
> +	struct net_device *netdev = data;
> +	struct e1000_adapter *adapter = netdev_priv(netdev);
> +	const int budget = 32; // FIXME should be auto-tuneable
> +	int tx_clean_complete = 0, work_done = 0;
> +
> +	while(!e1000_wait_for_intr(adapter)) {
> +		do {
> +			work_done = 0;
> +
> +			tx_clean_complete = e1000_clean_tx_irq(adapter, &adapter->tx_ring[0]);
> +			adapter->clean_rx(adapter, &adapter->rx_ring[0], &work_done, budget);
> +			if (!tx_clean_complete)
> +				work_done = budget;
> +
> +			if (work_done == budget) {
> +				/*
> +				 * Give up the rest of the timeslice to allow
> +				 * userspace to make forward progress
> +				 */
> +				sys_sched_yield();

sched_yield literally has undefined semantics if you are in SCHED_OTHER
priority (which your thread is). sched_yield use in userspace and even
kernel has caused headaches whenever the scheduler changes
significantly.

Please use anything but sched_yield. cond_resched() looks appropriate
here, it gives a simple resched point on !PREEMPT kernels. Then the
scheduler nice levels and newer resource controls should give sufficient
flexibility to fine tune CPU allocation after that.


> +			}
> +		} while (work_done == budget);
> +
> +		/* If budget not fully consumed, wait for an interrupt */
> +		adapter->last_icr = 0;
> +		if (likely(adapter->itr_setting & 3))
> +			e1000_set_itr(adapter);
> +		if (!test_bit(__E1000_DOWN, &adapter->flags))
> +			e1000_irq_enable(adapter);
> +	}
> +	return 0;
> +}

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html