Messages in this thread | | | Date | Sun, 31 Aug 2003 10:28:19 +0200 | From | Manfred Spraul <> | Subject | Re: [PATCH] Fix SMP support on 3c527 net driver |
| |
Felipe wrote:
>Also, the down/up function doesn't seem to be >used in interrupt context, so I think it will work. > > [snip]
> static int mc32_send_packet(struct sk_buff *skb, struct net_device *dev) > { > struct mc32_local *lp = (struct mc32_local *)dev->priv; >- unsigned long flags; > > volatile struct skb_header *p, *np; > > netif_stop_queue(dev); > >- save_flags(flags); >- cli(); >+ down(&lp->mc32_sem); > > No, that's wrong. mc32_send_packet is the hard_start_xmit function, called from bottom half context, with the dev_xmit_lock spinlock held. Additionally, you must replace the sleep_on calls with wait_event, or an open-coded wait queue: sleep_on is racy, it only works with cli().
IMHO the right way to fix cli() is - add a single spinlock to the driver or the device structure. Do not forget the spin_lock_init(). - replace cli/sti with spin_lock_irqsave/spin_unlock_irqsave. - Additionally acquire the spinlock in every interrupt handler (cli() stops all interrupts, spinlocks only stop interrupt on the current cpu). - check if there were recursive cli() calls. Fix them. - replace all sleep_on calls with wait queue calls. - check if there are any kmalloc or schedule calls in the area now under the spinlock, and reorganize the code.
And please add a changelog entry that code was converted without testing.
-- Manfred
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |