Thread (28 messages) 28 messages, 7 authors, 2011-06-23

Re: [RFT PATCH 7/9] ethtool: prepare for larger netdev_features_t type

From: Ben Hutchings <hidden>
Date: 2011-06-20 21:17:01

On Mon, 2011-06-20 at 21:14 +0200, Michał Mirosław wrote:
[...]
quoted hunk ↗ jump to hunk
@@ -125,19 +131,26 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
 	if (copy_from_user(features, useraddr, sizeof(features)))
 		return -EFAULT;
 
-	if (features[0].valid & ~NETIF_F_ETHTOOL_BITS)
+	/* I wonder if the compiler will be smart enough to loop-unroll
+	 * and optimize this... (no worries if not) --mq */
+	for (i = ETHTOOL_DEV_FEATURE_WORDS; i-- > 0; ) {
+		valid = (valid << 32)|features[i].valid;
+		wanted = (wanted << 32)|features[i].requested;
+	}
[...]

I don't know (or care) about optimisation of this, but I would expect
gcc to complain about shifting a 32-bit value by 32 bits.  I suggest you
write this as:

	for (i = 0; i < ETHTOOL_DEV_FEATURE_WORDS; ++i) {
		valid |= (netdev_features_t)features[i].valid << 32 *i;
		wanted |= (netdev_features_t)features[i].requested << 32 *i;
	}

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help