Jesse, This patch does need refining. There was one call to smp_processor_id() and one call to get_cpu(), so something needs to be changed for at least the smp_processor_id() case below.
But when get_cpu() does the preempt_disable(), where does the preempt_enable get invoked? Andy -----Original Message----- From: Brandeburg, Jesse [mailto:[email protected]] Sent: Thursday, October 21, 2010 2:44 PM To: Andy Cress Cc: [email protected] Subject: Re: [E1000-devel] [PATCH] fix ixgbe preempt bug On Thu, 21 Oct 2010, Andy Cress wrote: > My kernel configuration had CONFIG_PREEMPT=y, and the ixgbe 3.0.12 > module produced the following error message when it was loaded. > BUG: using smp_processor_id() in preemptible [00000000] code: > Below is a patch that works for me to resolve this. Please review it. > this patch is not necessary for any production kernel.org kernels. I'm not sure what kernel you're running, but maybe they have changed include/linux/smp.h for some reason to change get_cpu. All get_cpu calls defined include a preempt_disable that I can see. one other option is that our kcompat code tries to work around kernels without get_cpu, could that be the issue? does your kernel not implement the LINUX_VERSION_CODE macro? our code has this which could trigger an override of get_cpu #if ( LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0) ) #undef get_cpu #define get_cpu() smp_processor_id() #undef put_cpu #define put_cpu() do { } while(0) but that should only be in effect for kernels < 2.6 (aka 2.4.X kernels) I suggest putting a #warning in the macro above and see if you hit it. > Andy > > Signed-off-by: cress <[email protected]> > --- > drivers/net/ixgbe/ixgbe_main.c | 22 ++++++++++++++++------ > 1 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_main.c > b/drivers/net/ixgbe/ixgbe_main.c > index 25135fe..e89f5b6 100644 > --- a/drivers/net/ixgbe/ixgbe_main.c > +++ b/drivers/net/ixgbe/ixgbe_main.c > @@ -677,10 +677,12 @@ static void ixgbe_update_tx_dca(struct > ixgbe_adapter *adapter, > static void ixgbe_update_dca(struct ixgbe_q_vector *q_vector) > { > struct ixgbe_adapter *adapter = q_vector->adapter; > - int cpu = get_cpu(); > + int cpu; > long r_idx; > int i; > > + preempt_disable(); > + cpu = smp_processor_id(); /*same as get_cpu()*/ > if (q_vector->cpu == cpu) > goto out_no_update; > > @@ -701,6 +703,7 @@ static void ixgbe_update_dca(struct ixgbe_q_vector > *q_vector) > q_vector->cpu = cpu; > out_no_update: > put_cpu(); > + preempt_enable(); > } > > static void ixgbe_setup_dca(struct ixgbe_adapter *adapter) > @@ -8015,7 +8018,10 @@ static void ixgbe_netpoll(struct net_device > *netdev) > static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff > *skb) > { > struct ixgbe_adapter *adapter = netdev_priv(dev); > - int txq = smp_processor_id(); > + int txq; > + > + preempt_disable(); > + txq = smp_processor_id(); > > #ifdef IXGBE_FCOE > if ((skb->protocol == __constant_htons(ETH_P_FCOE)) || > @@ -8023,10 +8029,10 @@ static u16 ixgbe_select_queue(struct net_device > *dev, struct sk_buff *skb) > if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) { > txq &= > (adapter->ring_feature[RING_F_FCOE].indices - 1); > txq += adapter->ring_feature[RING_F_FCOE].mask; > - return txq; > + goto out_txq; > } else if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) { > txq = adapter->fcoe.up; > - return txq; > + goto out_txq; > } > } > > @@ -8034,7 +8040,7 @@ static u16 ixgbe_select_queue(struct net_device > *dev, struct sk_buff *skb) > if (adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE) { > while (unlikely(txq >= dev->real_num_tx_queues)) > txq -= dev->real_num_tx_queues; > - return txq; > + goto out_txq; > } > > if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) { > @@ -8043,9 +8049,13 @@ static u16 ixgbe_select_queue(struct net_device > *dev, struct sk_buff *skb) > else > txq = (skb->vlan_tci & > IXGBE_TX_FLAGS_VLAN_PRIO_MASK) > >> 13; > - return txq; > + goto out_txq; > } > + preempt_enable(); > return skb_tx_hash(dev, skb); > +out_txq: > + preempt_enable(); > + return txq; > } > > #endif /* HAVE_NETDEV_SELECT_QUEUE */ > ------------------------------------------------------------------------------ Nokia and AT&T present the 2010 Calling All Innovators-North America contest Create new apps & games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store http://p.sf.net/sfu/nokia-dev2dev _______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
