From: David M. <da...@co...> - 2002-01-18 16:22:36
|
> da...@co... said: > > The attached patch is a somewhat ugly fix to my problem of having the > > network interface hang. I'll describe the actual bug in case you want > > to fix it in a more elegant way. > > > The bug boils down to the fact that poll(2) will overwrite the -> > > revents field even when the ->events field was 0 going into the poll > > call. See? > > Cool. Nice bug hunt and nice analysis. I hate the fix :-) > > Try the patch below and see if it fixes it as well. It survived a 2M packet > ping flood with a find over the whole filesystem at the same time. > I have nearly the exact same (much prettier than my first one, eh?) fix here. It works fine. The first was a hack and slash effort, but it was sure to work, and I wanted to send something out ASAP. Here are some comments: - I think in the assignment to irq_fd->current_events you should assign the ->revents field, not the ->events field. Although it's just a boolean flag now, it makes sense given the name, the old code, and the fact that at some point the actual events returned by poll might want to be used in some way. - I still think there's a bug in the net_kern.c if the skb can't be allocated, we return without having done reactivate_fd. We'll never process the IRQ again, right? There may be other error return paths that have this bug. I haven't audited any others. - Under what circumstances will an IRQ handler legitimately not reactivate_fd? If there are none, why can't we use 'pollfds->events == 0' as the 'work is still to be done' flag (ditching the newly added irq_fd->current_events). Doing this will FIX the net_kern.c bug BTW and any others like it (i.e returning early from the do_IRQ handler before calling reactivate_fd) since the work will still be marked as 'TODO' until reactivate_fd is called. I ran a kernel with this and it worked fine, but I had a printk right after do_IRQ to test if do_IRQ ever returned without the reactivate_fd, and one particular driver, the console (stdin) does trigger it. I haven't look into this. David -- /==============================\ | David Mansfield | | da...@co... | \==============================/ |