==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Mitch Williams <[EMAIL PROTECTED]> adds:
mitch> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote: >> I've been speaking about this fix with a Jeff Moyer, and we've come up with >> some >> concerns regarding its implementation. Specifically the call to adapter-> clean_rx in the case of the e1000 driver is rather a layering >> violation in the netpoll code, in the sense that the function pointed to by >> clean_rx >> is functionality that is nominally used by the dev->poll method. In fact in >> this case, it would appear possible since dev->poll is called under the >> poll_lock, but dev->poll_controller is not, that is is possible to have cpus >> in >> a system executing in e1000_clean_rx_irq_[ps] at the same time leading to >> data >> corruption: >> >> CPU0: >> netpoll_poll_dev dev-> poll_controller (e1000_netpoll) adapter-> clean_rx (e1000_clean_rx_irq) >> >> CPU1: >> napi_poll dev-> poll (e1000_clean) >> e1000_clean_rx_irq mitch> Hmmm. You may have a point. I don't think a spinlock is required, but mitch> we do need to check if the poll is already scheduled on another CPU, mitch> like netpoll does in poll_napi(). mitch> In practice, of course, we never see this. The only netpoll client in mitch> the kernel is netconsole, which doesn't do receives. A few Major mitch> Distros use netdump, which does do receives, but only after the system mitch> has crashed. In that case, all other CPUs are stopped anyway. Don't forget about driver/net/kgdb_eth.c, which is in-tree, and does sends and receives. -Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
