Re: [patch] NE2000
Hi, I have to own up and say that it was me :-) you'll see that DECnet is the only protocol to use these macros at the moment. I'm sure though that I only copied what IPv4 was doing at the time, along with the hints I had from yourself and Dave, Steve. > > Hello! > > > Alexey! Even someone understood all this already, look > > to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-) > > > > I will compose a patch to fix all this. > > O! But who was this wiseman? 8) > > Alexey > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > Please read the FAQ at http://www.tux.org/lkml/ > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! > Alexey! Even someone understood all this already, look > to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-) > > I will compose a patch to fix all this. O! But who was this wiseman? 8) Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! Alexey! Even someone understood all this already, look to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-) I will compose a patch to fix all this. O! But who was this wiseman? 8) Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hi, I have to own up and say that it was me :-) you'll see that DECnet is the only protocol to use these macros at the moment. I'm sure though that I only copied what IPv4 was doing at the time, along with the hints I had from yourself and Dave, Steve. Hello! Alexey! Even someone understood all this already, look to include/net/sock.h SOCK_SLEEP_{PRE,POST} macros :-) I will compose a patch to fix all this. O! But who was this wiseman? 8) Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! > > In any case, Andrew, where is the race, when we enter in sleeping state? > > Wakeup is not lost, it is just not required when we are not going > > to schedule and force task to running state. > > set_current_state(TASK_INTERRUPTIBLE); > add_wait_queue(...); > /* window here */ > set_current_state(TASK_INTERRUPTIBLE); > schedule(); > > If there's a wakeup by another CPU (or this CPU in an interrupt) in > that window, current->state will get switched to TASK_RUNNING. > > Then it's immediately overwritten and we go to sleep. Lost wakeup. Look into code yet. It looks sort of different. Again: > > Wakeup is not lost, it is just not required when we are not going > > to schedule and force task to running state. So that it is right not depening on anything. Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! In any case, Andrew, where is the race, when we enter in sleeping state? Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(...); /* window here */ set_current_state(TASK_INTERRUPTIBLE); schedule(); If there's a wakeup by another CPU (or this CPU in an interrupt) in that window, current-state will get switched to TASK_RUNNING. Then it's immediately overwritten and we go to sleep. Lost wakeup. Look into code yet. It looks sort of different. Again: Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. So that it is right not depening on anything. Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
[EMAIL PROTECTED] wrote: > > Hello! > > > No, that code is correct, provided (current->state == TASK_RUNNING) > > on entry. If it isn't, there's a race window which can cause > > lost wakeups. As a check you could add: > > > > if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) > > BUG(); > > Though it really cannot happen and really happens, as we have seen... 8) > > In any case, Andrew, where is the race, when we enter in sleeping state? > Wakeup is not lost, it is just not required when we are not going > to schedule and force task to running state. > > I still do not see how it is possible that task runs in sleeping state. > Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) > OK, there are a few areas which look fishy. Calling __lock_sock when we're getting ready to wait on a different waitqueue looks like a rather risky area. We have a single task which is on two waitqueues. Consider the case of tcp_data_wait(): add_wait_queue(sk->sleep) set_current_state(TASK_INTERRUPTIBLE); release_sock(sk); if (...)/* Suppose this evaluates to false */ schedule_timeout(); lock_sock(); __lock_sock() { add_wait_queue_exclusive(sk->lock.wq); /* Window 1: What does a wake_up(sk->sleep) do here? */ current->state = TASK_EXCLUSIVE | TASK_UNINTERRUPTIBLE; /* Window 2: Bad things happen here */ schedule(); If someone does a wakeup(sk->sleep) in Window 2 in __lock_sock() the wakeup code will think that the task is sleeping on sk->sleep in state TASK_EXCLUSIVE|TASK_UNINTERRUPTIBLE, when in fact it is not. So a wakeup which _should_ have gone to a different exclusive task actually goes to this one. This is fantastically hard to hit because of the direction of the waitqueue scan. If the wakeup on sk->sleep happens during Window 1 it will be completely lost, but that's OK because this task is not yet TASK_EXCLUSIVE (providing the write ordering behaves as we want?) If a wakeup on sk->lock.wq happens during Window 1 it will be completely lost. wait_for_connect() and wait_for_tcp_memory() play similar games with lock_sock() whereby they can appear to be on two waitqueues at the same time. And again, because lock_sock() uses TASK_EXCLUSIVE a wake_up on sk->sleep could choose this task instead of a TASK_EXCLUSIVE task which is _really_ sleeping on sk->sleep. Now, this may not be a problem in practise, and in fact the above may not be bugs because I missed something. But I suggest you have a think about it. My brain is starting to hurt. But none of these explain Jorge's problem. How he got to where he did in !TASK_RUNNING. Plus the possible lock_sock problems just look too damn hard to hit to explain Jorge's repeatability. It may be useful to put a Pentium hardware watchpoint onto current->state. Does kdb support those? Can sock_fasync() be called when we're on a waitqueue, not in state TASK_RUNNING and prior to schedule()? inet_wait_for_connect() is OK. wait_for_tcp_connect() is OK. tcp_close() is OK. Also, are you sure that all occurrences of current->state = ; are still safe on weakly ordered CPUs? (Not that this would explain Jorge's problem). hmm.. khttpd tries to do wake-one, but interruptible_sleep_on_timeout() confounds it. Bummer. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
[EMAIL PROTECTED] wrote: > > Hello! > > > No, that code is correct, provided (current->state == TASK_RUNNING) > > on entry. If it isn't, there's a race window which can cause > > lost wakeups. As a check you could add: > > > > if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) > > BUG(); > > Though it really cannot happen and really happens, as we have seen... 8) > > In any case, Andrew, where is the race, when we enter in sleeping state? > Wakeup is not lost, it is just not required when we are not going > to schedule and force task to running state. set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(...); /* window here */ set_current_state(TASK_INTERRUPTIBLE); schedule(); If there's a wakeup by another CPU (or this CPU in an interrupt) in that window, current->state will get switched to TASK_RUNNING. Then it's immediately overwritten and we go to sleep. Lost wakeup. > I still do not see how it is possible that task runs in sleeping state. > Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) Nope. Is Jorge running SMP? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! > No, that code is correct, provided (current->state == TASK_RUNNING) > on entry. If it isn't, there's a race window which can cause > lost wakeups. As a check you could add: > > if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) > BUG(); Though it really cannot happen and really happens, as we have seen... 8) In any case, Andrew, where is the race, when we enter in sleeping state? Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. I still do not see how it is possible that task runs in sleeping state. Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! > if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) > == 0) > BUG(); The Puzzle... 8) It is truly impossible. Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Andrew Morton wrote: > > Jorge Nerin wrote: > > > > ... > > So I think that it could be a little window near sock_wait_for_wmem that > > could be SMP insecure wich is affecting me. > > > > The code of sock_wait_for_wmem in 2.4.0-test10 is this: > > > > static long sock_wait_for_wmem(struct sock * sk, long timeo) > > { > > DECLARE_WAITQUEUE(wait, current); > > > > clear_bit(SOCK_ASYNC_NOSPACE, >socket->flags); > > add_wait_queue(sk->sleep, ); > > for (;;) { > > if (signal_pending(current)) > > break; > > set_bit(SOCK_NOSPACE, >socket->flags); > > set_current_state(TASK_INTERRUPTIBLE); > > if (atomic_read(>wmem_alloc) < sk->sndbuf) > > break; > > if (sk->shutdown & SEND_SHUTDOWN) > > break; > > if (sk->err) > > break; > > timeo = schedule_timeout(timeo); > > } > > __set_current_state(TASK_RUNNING); > > remove_wait_queue(sk->sleep, ); > > return timeo; > > } > > > > Does someone see something SMP insecure? Perhaps I'm totally wrong, this > > could also be somewhere in the interrupt handling, don't know. > > No, that code is correct, provided (current->state == TASK_RUNNING) > on entry. If it isn't, there's a race window which can cause > lost wakeups. As a check you could add: > > if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) > BUG(); > > to the start of this function. OK, added, the function now looks like this: static long sock_wait_for_wmem(struct sock * sk, long timeo) { DECLARE_WAITQUEUE(wait, current); if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); clear_bit(SOCK_ASYNC_NOSPACE, >socket->flags); add_wait_queue(sk->sleep, ); for (;;) { if (signal_pending(current)) break; set_bit(SOCK_NOSPACE, >socket->flags); set_current_state(TASK_INTERRUPTIBLE); if (atomic_read(>wmem_alloc) < sk->sndbuf) break; if (sk->shutdown & SEND_SHUTDOWN) break; if (sk->err) break; timeo = schedule_timeout(timeo); } __set_current_state(TASK_RUNNING); remove_wait_queue(sk->sleep, ); return timeo; } I have to put it _after_ DECLARE_WAITQUEUE in order to compile, if I put it before it says me that wait is undeclared :-? Well recompile, reboot, and got caugth by BUG(), after some tests. [root@quartz ~]# ping -f -s 15000 pp_head PING pp_head.pp.redvip.net (192.168.1.20) from 192.168.1.3 : 15000(15028) bytes of data. ..invalid operand: CPU:0 EIP:0010:[] EFLAGS: 00010296 eax: 001a ebx: c2604000 ecx: c021e2e8 edx: c0266440 esi: c26eeba0 edi: c26eeba0 ebp: 7fff esp: c2605c88 ds: 0018 es: 0018 ss: 0018 Process ping (pid: 2202, stackpage=c2605000) Stack: c02047a5 c02049eb 02d2 7fff c26eeba0 c2604000 c26c4024 01234567 c2604000 01234567 c2604000 c0195c99 c26eeba0 7fff c26c4024 c264d0c0 c26c4010 05dc Call Trace: [] [] [] [] [] [] [] [] [] [] [] [] [] [] [] [] [] [] Code: 0f 0b 83 c4 0c 8b 87 50 03 00 00 f0 0f ba 70 04 00 8d 4c 24 ViolaciĆ³n de segmento [root@quartz ~]# Nov 6 12:00:07 quartz kernel: kernel BUG at sock.c:722! Nov 6 12:00:07 quartz kernel: invalid operand: Nov 6 12:00:07 quartz kernel: CPU:0 Nov 6 12:00:07 quartz kernel: EIP:0010:[sock_wait_for_wmem+104/244] Nov 6 12:00:07 quartz kernel: EFLAGS: 00010296 Nov 6 12:00:07 quartz kernel: eax: 001a ebx: c2604000 ecx: c021e2e8 edx: c0266440 Nov 6 12:00:07 quartz kernel: esi: c26eeba0 edi: c26eeba0 ebp: 7fff esp: c2605c88 Nov 6 12:00:07 quartz kernel: ds: 0018 es: 0018 ss: 0018 Nov 6 12:00:07 quartz kernel: Process ping (pid: 2202, stackpage=c2605000) Nov 6 12:00:07 quartz kernel: Stack: c02047a5 c02049eb 02d2 7fff c26eeba0 c2604000 c26c4024 Nov 6 12:00:07 quartz kernel:01234567 c2604000 01234567 c2604000 Nov 6 12:00:07 quartz kernel:c0195c99 c26eeba0 7fff c26c4024 c264d0c0 c26c4010 05dc Nov 6 12:00:07 quartz kernel: Call Trace: [vga_con+2501/10176] [vga_con+3083/10176] [sock_alloc_send_skb+221/300] [ip_build_xmit_slow+378/1208] [ip_build_xmit+78/816] [raw_getfrag+0/36] [raw_sendmsg+642/752] Nov 6 12:00:07 quartz kernel:[raw_getfrag+0/36] [inet_sendmsg+0/68] [inet_sendmsg+62/68] [sock_sendmsg+129/164] [inet_sendmsg+0/68] [sys_sendmsg+380/464] [sys_recvfrom+238/256] [set_cursor+110/132] Nov 6 12:00:07 quartz kernel:[write_chan+462/488]
Re: [patch] NE2000
Hello! if ((current-state (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); The Puzzle... 8) It is truly impossible. Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Hello! No, that code is correct, provided (current-state == TASK_RUNNING) on entry. If it isn't, there's a race window which can cause lost wakeups. As a check you could add: if ((current-state (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); Though it really cannot happen and really happens, as we have seen... 8) In any case, Andrew, where is the race, when we enter in sleeping state? Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. I still do not see how it is possible that task runs in sleeping state. Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) Alexey - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
[EMAIL PROTECTED] wrote: Hello! No, that code is correct, provided (current-state == TASK_RUNNING) on entry. If it isn't, there's a race window which can cause lost wakeups. As a check you could add: if ((current-state (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); Though it really cannot happen and really happens, as we have seen... 8) In any case, Andrew, where is the race, when we enter in sleeping state? Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(...); /* window here */ set_current_state(TASK_INTERRUPTIBLE); schedule(); If there's a wakeup by another CPU (or this CPU in an interrupt) in that window, current-state will get switched to TASK_RUNNING. Then it's immediately overwritten and we go to sleep. Lost wakeup. I still do not see how it is possible that task runs in sleeping state. Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) Nope. Is Jorge running SMP? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
[EMAIL PROTECTED] wrote: Hello! No, that code is correct, provided (current-state == TASK_RUNNING) on entry. If it isn't, there's a race window which can cause lost wakeups. As a check you could add: if ((current-state (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); Though it really cannot happen and really happens, as we have seen... 8) In any case, Andrew, where is the race, when we enter in sleeping state? Wakeup is not lost, it is just not required when we are not going to schedule and force task to running state. I still do not see how it is possible that task runs in sleeping state. Apparently, set_current_state is forgotten somewhere. Do you see, where? 8) OK, there are a few areas which look fishy. Calling __lock_sock when we're getting ready to wait on a different waitqueue looks like a rather risky area. We have a single task which is on two waitqueues. Consider the case of tcp_data_wait(): add_wait_queue(sk-sleep) set_current_state(TASK_INTERRUPTIBLE); release_sock(sk); if (...)/* Suppose this evaluates to false */ schedule_timeout(); lock_sock(); __lock_sock() { add_wait_queue_exclusive(sk-lock.wq); /* Window 1: What does a wake_up(sk-sleep) do here? */ current-state = TASK_EXCLUSIVE | TASK_UNINTERRUPTIBLE; /* Window 2: Bad things happen here */ schedule(); If someone does a wakeup(sk-sleep) in Window 2 in __lock_sock() the wakeup code will think that the task is sleeping on sk-sleep in state TASK_EXCLUSIVE|TASK_UNINTERRUPTIBLE, when in fact it is not. So a wakeup which _should_ have gone to a different exclusive task actually goes to this one. This is fantastically hard to hit because of the direction of the waitqueue scan. If the wakeup on sk-sleep happens during Window 1 it will be completely lost, but that's OK because this task is not yet TASK_EXCLUSIVE (providing the write ordering behaves as we want?) If a wakeup on sk-lock.wq happens during Window 1 it will be completely lost. wait_for_connect() and wait_for_tcp_memory() play similar games with lock_sock() whereby they can appear to be on two waitqueues at the same time. And again, because lock_sock() uses TASK_EXCLUSIVE a wake_up on sk-sleep could choose this task instead of a TASK_EXCLUSIVE task which is _really_ sleeping on sk-sleep. Now, this may not be a problem in practise, and in fact the above may not be bugs because I missed something. But I suggest you have a think about it. My brain is starting to hurt. But none of these explain Jorge's problem. How he got to where he did in !TASK_RUNNING. Plus the possible lock_sock problems just look too damn hard to hit to explain Jorge's repeatability. It may be useful to put a Pentium hardware watchpoint onto current-state. Does kdb support those? Can sock_fasync() be called when we're on a waitqueue, not in state TASK_RUNNING and prior to schedule()? inet_wait_for_connect() is OK. wait_for_tcp_connect() is OK. tcp_close() is OK. Also, are you sure that all occurrences of current-state = whatever; are still safe on weakly ordered CPUs? (Not that this would explain Jorge's problem). hmm.. khttpd tries to do wake-one, but interruptible_sleep_on_timeout() confounds it. Bummer. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Jorge Nerin wrote: > > ... > So I think that it could be a little window near sock_wait_for_wmem that > could be SMP insecure wich is affecting me. > > The code of sock_wait_for_wmem in 2.4.0-test10 is this: > > static long sock_wait_for_wmem(struct sock * sk, long timeo) > { > DECLARE_WAITQUEUE(wait, current); > > clear_bit(SOCK_ASYNC_NOSPACE, >socket->flags); > add_wait_queue(sk->sleep, ); > for (;;) { > if (signal_pending(current)) > break; > set_bit(SOCK_NOSPACE, >socket->flags); > set_current_state(TASK_INTERRUPTIBLE); > if (atomic_read(>wmem_alloc) < sk->sndbuf) > break; > if (sk->shutdown & SEND_SHUTDOWN) > break; > if (sk->err) > break; > timeo = schedule_timeout(timeo); > } > __set_current_state(TASK_RUNNING); > remove_wait_queue(sk->sleep, ); > return timeo; > } > > Does someone see something SMP insecure? Perhaps I'm totally wrong, this > could also be somewhere in the interrupt handling, don't know. No, that code is correct, provided (current->state == TASK_RUNNING) on entry. If it isn't, there's a race window which can cause lost wakeups. As a check you could add: if ((current->state & (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); to the start of this function. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul Gortmaker wrote: > > Jorge Nerin wrote: > > > > > Ok, I reported it several times, but it gets ignored. I have a Realtek > > 8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get > > it stuck by doing a ping -f to a host in the local net, and sometimes it > > happens doing copies to/from nfs shared resources. > > > > rmmod & insmod don't cure the problem, it seems that no interrupts are > > delivered from the card, and there are no log messages, so a reboot is > > needed to restore net access. > > > > System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far > > as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit > > this problem. > > Any messages from the driver whatsoever? Does running a non-SMP > kernel make the problem go away? > > Paul. > Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the result is a little confusing. Under SMP a ping -s 5 -f other_host takes down the network access with no messages (ne2k-pci), and no possibility of being restored without a reboot. Under UP the same command works ok, but after a while the dots stop for 30sec, then ping prints an 'E' and the dots continue. strace revealed this: sendmsg(4, {msg_name(16)={sin_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("192.168.1.20")}}, msg_iov(1)=[{"\10\0\305~|\23\231\0\v\317\2:\177\236\r\0\10\t\n\v\f\r"..., 50008}], msg_controllen=0, msg_flags=0}, 0x800) = 50008 <30.016855> ping has been waiting for sendmsg to end for 30 seconds! I don't know if this could be caused by filling the network buffers, and then waiting a while while the nic sends it out. As the packet size increases (the -s option) the stops are more frequent, there is still activity on the network, but I don't know if they are my packets or the replies. When ping is stopped it's stuck in sock_wait_for_wmem, and when it's running it's (usually) in wait_for_packet. So I think that it could be a little window near sock_wait_for_wmem that could be SMP insecure wich is affecting me. The code of sock_wait_for_wmem in 2.4.0-test10 is this: static long sock_wait_for_wmem(struct sock * sk, long timeo) { DECLARE_WAITQUEUE(wait, current); clear_bit(SOCK_ASYNC_NOSPACE, >socket->flags); add_wait_queue(sk->sleep, ); for (;;) { if (signal_pending(current)) break; set_bit(SOCK_NOSPACE, >socket->flags); set_current_state(TASK_INTERRUPTIBLE); if (atomic_read(>wmem_alloc) < sk->sndbuf) break; if (sk->shutdown & SEND_SHUTDOWN) break; if (sk->err) break; timeo = schedule_timeout(timeo); } __set_current_state(TASK_RUNNING); remove_wait_queue(sk->sleep, ); return timeo; } Does someone see something SMP insecure? Perhaps I'm totally wrong, this could also be somewhere in the interrupt handling, don't know. -- Jorge Nerin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul Gortmaker wrote: Jorge Nerin wrote: Ok, I reported it several times, but it gets ignored. I have a Realtek 8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get it stuck by doing a ping -f to a host in the local net, and sometimes it happens doing copies to/from nfs shared resources. rmmod insmod don't cure the problem, it seems that no interrupts are delivered from the card, and there are no log messages, so a reboot is needed to restore net access. System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit this problem. Any messages from the driver whatsoever? Does running a non-SMP kernel make the problem go away? Paul. Well, I have tried it with 2.4.0-test10, both SMP and non-SMP, and the result is a little confusing. Under SMP a ping -s 5 -f other_host takes down the network access with no messages (ne2k-pci), and no possibility of being restored without a reboot. Under UP the same command works ok, but after a while the dots stop for 30sec, then ping prints an 'E' and the dots continue. strace revealed this: sendmsg(4, {msg_name(16)={sin_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("192.168.1.20")}}, msg_iov(1)=[{"\10\0\305~|\23\231\0\v\317\2:\177\236\r\0\10\t\n\v\f\r"..., 50008}], msg_controllen=0, msg_flags=0}, 0x800) = 50008 30.016855 ping has been waiting for sendmsg to end for 30 seconds! I don't know if this could be caused by filling the network buffers, and then waiting a while while the nic sends it out. As the packet size increases (the -s option) the stops are more frequent, there is still activity on the network, but I don't know if they are my packets or the replies. When ping is stopped it's stuck in sock_wait_for_wmem, and when it's running it's (usually) in wait_for_packet. So I think that it could be a little window near sock_wait_for_wmem that could be SMP insecure wich is affecting me. The code of sock_wait_for_wmem in 2.4.0-test10 is this: static long sock_wait_for_wmem(struct sock * sk, long timeo) { DECLARE_WAITQUEUE(wait, current); clear_bit(SOCK_ASYNC_NOSPACE, sk-socket-flags); add_wait_queue(sk-sleep, wait); for (;;) { if (signal_pending(current)) break; set_bit(SOCK_NOSPACE, sk-socket-flags); set_current_state(TASK_INTERRUPTIBLE); if (atomic_read(sk-wmem_alloc) sk-sndbuf) break; if (sk-shutdown SEND_SHUTDOWN) break; if (sk-err) break; timeo = schedule_timeout(timeo); } __set_current_state(TASK_RUNNING); remove_wait_queue(sk-sleep, wait); return timeo; } Does someone see something SMP insecure? Perhaps I'm totally wrong, this could also be somewhere in the interrupt handling, don't know. -- Jorge Nerin [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Jorge Nerin wrote: ... So I think that it could be a little window near sock_wait_for_wmem that could be SMP insecure wich is affecting me. The code of sock_wait_for_wmem in 2.4.0-test10 is this: static long sock_wait_for_wmem(struct sock * sk, long timeo) { DECLARE_WAITQUEUE(wait, current); clear_bit(SOCK_ASYNC_NOSPACE, sk-socket-flags); add_wait_queue(sk-sleep, wait); for (;;) { if (signal_pending(current)) break; set_bit(SOCK_NOSPACE, sk-socket-flags); set_current_state(TASK_INTERRUPTIBLE); if (atomic_read(sk-wmem_alloc) sk-sndbuf) break; if (sk-shutdown SEND_SHUTDOWN) break; if (sk-err) break; timeo = schedule_timeout(timeo); } __set_current_state(TASK_RUNNING); remove_wait_queue(sk-sleep, wait); return timeo; } Does someone see something SMP insecure? Perhaps I'm totally wrong, this could also be somewhere in the interrupt handling, don't know. No, that code is correct, provided (current-state == TASK_RUNNING) on entry. If it isn't, there's a race window which can cause lost wakeups. As a check you could add: if ((current-state (TASK_INTERRUPTIBLE|TASK_UNINTERRUPTIBLE)) == 0) BUG(); to the start of this function. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul, Ok, here's what I have. Included are your changes, as well as drivers/net/ne.c: * use probe_irq_on/off instead of autoirq_xxx (autoirq is going away) * request_region first thing in ne_probe1, before any hardware interaction takes place. Eliminates any potential resource races. Also eliminates a call to check_region. * Trim trailing whitespace drivers/net/ne2k-pci.c: * Merge Becker version 1.02 ne2k-pci.c, which adds forced full duplex support, and also several cosmetic changes where merely serve to bring the kernel's ne2k-pci.c closer to Becker's version (ie. makes the diff smaller). * Just call BUG() if we don't have a net_device in ne2k_pci_remove_one * Correct pci_module_init return code handling As an aside, for 2.5.x, would it be possible to merge all the common ne2000 code (block_input/output, etc.) from the various ne2k drivers? As it stands there exists ne.c, ne2.c, ne2k-pci.c, and pcnet_cs.c, all of which do pretty much the same thing at their core. [and AFAICS, all but pcnet_cs might easily call a common ne2k library] Regards, Jeff -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max Index: drivers/net/ne.c === RCS file: /cvsroot/gkernel/linux_2_4/drivers/net/ne.c,v retrieving revision 1.1.1.4 diff -u -r1.1.1.4 ne.c --- drivers/net/ne.c2000/10/31 21:21:49 1.1.1.4 +++ drivers/net/ne.c2000/11/01 13:24:36 @@ -29,21 +29,22 @@ occur after memory is allocated for dev->priv. Deallocated memory last in cleanup_modue() Richard Guenther: Added support for ISAPnP cards - +Paul Gortmaker : Discontinued PCI support - use ne2k-pci.c instead. + */ /* Routines for the NatSemi-based designs (NE[12]000). */ -static const char *version = -"ne.c:v1.10 9/23/94 Donald Becker ([EMAIL PROTECTED])\n"; +static const char version1[] = +"ne.c:v1.10 9/23/94 Donald Becker ([EMAIL PROTECTED])\n"; +static const char version2[] = +"Last modified Nov 1, 2000 by Paul Gortmaker\n"; #include -#include #include #include #include -#include #include #include #include @@ -70,28 +71,11 @@ /* A zero-terminated list of I/O addresses to be probed at boot. */ #ifndef MODULE -static unsigned int netcard_portlist[] __initdata = { +static unsigned int netcard_portlist[] __initdata = { 0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0 }; #endif -#ifdef CONFIG_PCI -/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */ -static struct { unsigned short vendor, dev_id; char *name; } -pci_clone_list[] __initdata = { - {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8029, "Realtek 8029" }, - {PCI_VENDOR_ID_WINBOND2,PCI_DEVICE_ID_WINBOND2_89C940, "Winbond 89C940" }, - {PCI_VENDOR_ID_COMPEX, PCI_DEVICE_ID_COMPEX_RL2000, "Compex ReadyLink 2000" }, - {PCI_VENDOR_ID_KTI, PCI_DEVICE_ID_KTI_ET32P2, "KTI ET32P2" }, - {PCI_VENDOR_ID_NETVIN, PCI_DEVICE_ID_NETVIN_NV5000SC, "NetVin NV5000" }, - {PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C926, "VIA 82C926 Amazon" }, - {PCI_VENDOR_ID_SURECOM, PCI_DEVICE_ID_SURECOM_NE34, "SureCom NE34"}, - {0,} -}; - -static int probe_pci = 1; -#endif - static struct { unsigned short vendor, function; char *name; } isapnp_clone_list[] __initdata = { {ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216), "NN NE2000" }, @@ -114,7 +98,9 @@ {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */ {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */ {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */ -{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */ +{"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */ {0,} }; #endif @@ -132,15 +118,9 @@ #define NESM_START_PG 0x40/* First page of TX buffer */ #define NESM_STOP_PG 0x80/* Last page +1 of RX ring */ -/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */ -static unsigned int pci_irq_line = 0; - int ne_probe(struct net_device *dev); static int ne_probe1(struct net_device *dev, int ioaddr); static int ne_probe_isapnp(struct net_device *dev); -#ifdef CONFIG_PCI -static int ne_probe_pci(struct net_device *dev); -#endif static int ne_open(struct net_device *dev); static int ne_close(struct net_device *dev); @@ -175,16 +155,9 @@ E2010starts at 0x100 and ends at 0x4000. E2010-x
Re: [patch] NE2000
On Wed, 1 Nov 2000, Paul Gortmaker wrote: > Jeff Garzik wrote: > > Paul Gortmaker wrote: > > > There is no urgency in trying to squeeze a patch like this in the back > > > door of a 2.4.0 release. For example, there are people out there now > > > who are using the ne.c driver to run both ISA and PCI cards in the same > > > box without having to use 2 different drivers. We can wait until 2.5.0 > > > to break their .config file. > > > > IMNSHO this is a bug, though... .. > If you want to roll it into the merge (and can get it past Linus) then > please feel free to do so - I'll be glad to cross it off my list sooner > as opposed to later. If the ne* drivers are going to be updated, you might want to add in the full-duplex support of the latest ne2k-pci.c driver at ftp://www.scyld.com/pub/network/ne2k-pci.c Donald Becker [EMAIL PROTECTED] Scyld Computing Corporation http://www.scyld.com 410 Severn Ave. Suite 210 Second Generation Beowulf Clusters Annapolis MD 21403 410-990-9993 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
On Wed, 1 Nov 2000, Paul Gortmaker wrote: Jeff Garzik wrote: Paul Gortmaker wrote: There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. IMNSHO this is a bug, though... .. If you want to roll it into the merge (and can get it past Linus) then please feel free to do so - I'll be glad to cross it off my list sooner as opposed to later. If the ne* drivers are going to be updated, you might want to add in the full-duplex support of the latest ne2k-pci.c driver at ftp://www.scyld.com/pub/network/ne2k-pci.c Donald Becker [EMAIL PROTECTED] Scyld Computing Corporation http://www.scyld.com 410 Severn Ave. Suite 210 Second Generation Beowulf Clusters Annapolis MD 21403 410-990-9993 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul, Ok, here's what I have. Included are your changes, as well as drivers/net/ne.c: * use probe_irq_on/off instead of autoirq_xxx (autoirq is going away) * request_region first thing in ne_probe1, before any hardware interaction takes place. Eliminates any potential resource races. Also eliminates a call to check_region. * Trim trailing whitespace drivers/net/ne2k-pci.c: * Merge Becker version 1.02 ne2k-pci.c, which adds forced full duplex support, and also several cosmetic changes where merely serve to bring the kernel's ne2k-pci.c closer to Becker's version (ie. makes the diff smaller). * Just call BUG() if we don't have a net_device in ne2k_pci_remove_one * Correct pci_module_init return code handling As an aside, for 2.5.x, would it be possible to merge all the common ne2000 code (block_input/output, etc.) from the various ne2k drivers? As it stands there exists ne.c, ne2.c, ne2k-pci.c, and pcnet_cs.c, all of which do pretty much the same thing at their core. [and AFAICS, all but pcnet_cs might easily call a common ne2k library] Regards, Jeff -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max Index: drivers/net/ne.c === RCS file: /cvsroot/gkernel/linux_2_4/drivers/net/ne.c,v retrieving revision 1.1.1.4 diff -u -r1.1.1.4 ne.c --- drivers/net/ne.c2000/10/31 21:21:49 1.1.1.4 +++ drivers/net/ne.c2000/11/01 13:24:36 @@ -29,21 +29,22 @@ occur after memory is allocated for dev-priv. Deallocated memory last in cleanup_modue() Richard Guenther: Added support for ISAPnP cards - +Paul Gortmaker : Discontinued PCI support - use ne2k-pci.c instead. + */ /* Routines for the NatSemi-based designs (NE[12]000). */ -static const char *version = -"ne.c:v1.10 9/23/94 Donald Becker ([EMAIL PROTECTED])\n"; +static const char version1[] = +"ne.c:v1.10 9/23/94 Donald Becker ([EMAIL PROTECTED])\n"; +static const char version2[] = +"Last modified Nov 1, 2000 by Paul Gortmaker\n"; #include linux/module.h -#include linux/config.h #include linux/kernel.h #include linux/sched.h #include linux/errno.h -#include linux/pci.h #include linux/isapnp.h #include linux/init.h #include linux/delay.h @@ -70,28 +71,11 @@ /* A zero-terminated list of I/O addresses to be probed at boot. */ #ifndef MODULE -static unsigned int netcard_portlist[] __initdata = { +static unsigned int netcard_portlist[] __initdata = { 0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0 }; #endif -#ifdef CONFIG_PCI -/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */ -static struct { unsigned short vendor, dev_id; char *name; } -pci_clone_list[] __initdata = { - {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8029, "Realtek 8029" }, - {PCI_VENDOR_ID_WINBOND2,PCI_DEVICE_ID_WINBOND2_89C940, "Winbond 89C940" }, - {PCI_VENDOR_ID_COMPEX, PCI_DEVICE_ID_COMPEX_RL2000, "Compex ReadyLink 2000" }, - {PCI_VENDOR_ID_KTI, PCI_DEVICE_ID_KTI_ET32P2, "KTI ET32P2" }, - {PCI_VENDOR_ID_NETVIN, PCI_DEVICE_ID_NETVIN_NV5000SC, "NetVin NV5000" }, - {PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C926, "VIA 82C926 Amazon" }, - {PCI_VENDOR_ID_SURECOM, PCI_DEVICE_ID_SURECOM_NE34, "SureCom NE34"}, - {0,} -}; - -static int probe_pci = 1; -#endif - static struct { unsigned short vendor, function; char *name; } isapnp_clone_list[] __initdata = { {ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216), "NN NE2000" }, @@ -114,7 +98,9 @@ {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */ {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */ {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */ -{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */ +{"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */ {0,} }; #endif @@ -132,15 +118,9 @@ #define NESM_START_PG 0x40/* First page of TX buffer */ #define NESM_STOP_PG 0x80/* Last page +1 of RX ring */ -/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */ -static unsigned int pci_irq_line = 0; - int ne_probe(struct net_device *dev); static int ne_probe1(struct net_device *dev, int ioaddr); static int ne_probe_isapnp(struct net_device *dev); -#ifdef CONFIG_PCI -static int ne_probe_pci(struct net_device *dev); -#endif static int ne_open(struct net_device *dev); static int
Re: [patch] NE2000
Jeff Garzik wrote: > > Paul Gortmaker wrote: > > There is no urgency in trying to squeeze a patch like this in the back > > door of a 2.4.0 release. For example, there are people out there now > > who are using the ne.c driver to run both ISA and PCI cards in the same > > box without having to use 2 different drivers. We can wait until 2.5.0 > > to break their .config file. > > IMNSHO this is a bug, though... Maybe I wasn't 100% clear - these are people who have intentionally chosen to do so. (There is probably a slight icache benefit in sharing the same driver like this, FWIW) > Since ne2k-pci.c supports all boards ne.c does, and includes some fixes > that ne.c does not, it seems like removing the PCI support in ne.c is a > bug fix change. If you want to roll it into the merge (and can get it past Linus) then please feel free to do so - I'll be glad to cross it off my list sooner as opposed to later. In addition to removing all remains of PCI support from ne.c this patch also has three trivial changes that were also in my queue for ne.c. 1) Two new ID signatures added to the bad_clone_list 2) int base_addr promoted to unsigned int base_addr in ne_probe, as required for SuperH CPU systems that have ne2000 cards. 3) ISA PnP card info printk'd *before* the actual probe, and not after (just in case the probe silently hangs or whatever). Paul. --- 2400-t10/linux-g/drivers/net/ne.c~ Tue Jul 11 02:29:10 2000 +++ 2400-t10/linux-g/drivers/net/ne.c Wed Nov 1 00:02:17 2000 @@ -29,6 +29,7 @@ occur after memory is allocated for dev->priv. Deallocated memory last in cleanup_modue() Richard Guenther: Added support for ISAPnP cards +Paul Gortmaker : Discontinued PCI support - use ne2k-pci.c instead. */ @@ -43,7 +44,6 @@ #include #include #include -#include #include #include #include @@ -75,23 +75,6 @@ }; #endif -#ifdef CONFIG_PCI -/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */ -static struct { unsigned short vendor, dev_id; char *name; } -pci_clone_list[] __initdata = { - {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8029, "Realtek 8029" }, - {PCI_VENDOR_ID_WINBOND2,PCI_DEVICE_ID_WINBOND2_89C940, "Winbond 89C940" }, - {PCI_VENDOR_ID_COMPEX, PCI_DEVICE_ID_COMPEX_RL2000, "Compex ReadyLink 2000" }, - {PCI_VENDOR_ID_KTI, PCI_DEVICE_ID_KTI_ET32P2, "KTI ET32P2" }, - {PCI_VENDOR_ID_NETVIN, PCI_DEVICE_ID_NETVIN_NV5000SC, "NetVin NV5000" }, - {PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C926, "VIA 82C926 Amazon" }, - {PCI_VENDOR_ID_SURECOM, PCI_DEVICE_ID_SURECOM_NE34, "SureCom NE34"}, - {0,} -}; - -static int probe_pci = 1; -#endif - static struct { unsigned short vendor, function; char *name; } isapnp_clone_list[] __initdata = { {ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216), "NN NE2000" }, @@ -114,7 +97,9 @@ {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */ {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */ {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */ -{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */ +{"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */ {0,} }; #endif @@ -132,15 +117,9 @@ #define NESM_START_PG 0x40/* First page of TX buffer */ #define NESM_STOP_PG 0x80/* Last page +1 of RX ring */ -/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */ -static unsigned int pci_irq_line = 0; - int ne_probe(struct net_device *dev); static int ne_probe1(struct net_device *dev, int ioaddr); static int ne_probe_isapnp(struct net_device *dev); -#ifdef CONFIG_PCI -static int ne_probe_pci(struct net_device *dev); -#endif static int ne_open(struct net_device *dev); static int ne_close(struct net_device *dev); @@ -180,16 +159,9 @@ {"ne", ne_probe1, NE_IO_EXTENT, netcard_portlist}; #else -/* - * Note that at boot, this probe only picks up one card at a time, even for - * multiple PCI ne2k cards. Use "ether=0,0,eth1" if you have a second PCI - * ne2k card. This keeps things consistent regardless of the bus type of - * the card. - */ - int __init ne_probe(struct net_device *dev) { - int base_addr = dev ? dev->base_addr : 0; + unsigned int base_addr = dev ? dev->base_addr : 0; /* First check any supplied i/o locations. User knows best. */ if (base_addr > 0x1ff) /* Check a single specified location. */ @@ -197,12 +169,6 @@ else if (base_addr != 0)/* Don't probe at all. */ return
Re: [patch] NE2000
Jeff Garzik wrote: Paul Gortmaker wrote: There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. IMNSHO this is a bug, though... Maybe I wasn't 100% clear - these are people who have intentionally chosen to do so. (There is probably a slight icache benefit in sharing the same driver like this, FWIW) Since ne2k-pci.c supports all boards ne.c does, and includes some fixes that ne.c does not, it seems like removing the PCI support in ne.c is a bug fix change. If you want to roll it into the merge (and can get it past Linus) then please feel free to do so - I'll be glad to cross it off my list sooner as opposed to later. In addition to removing all remains of PCI support from ne.c this patch also has three trivial changes that were also in my queue for ne.c. 1) Two new ID signatures added to the bad_clone_list 2) int base_addr promoted to unsigned int base_addr in ne_probe, as required for SuperH CPU systems that have ne2000 cards. 3) ISA PnP card info printk'd *before* the actual probe, and not after (just in case the probe silently hangs or whatever). Paul. --- 2400-t10/linux-g/drivers/net/ne.c~ Tue Jul 11 02:29:10 2000 +++ 2400-t10/linux-g/drivers/net/ne.c Wed Nov 1 00:02:17 2000 @@ -29,6 +29,7 @@ occur after memory is allocated for dev-priv. Deallocated memory last in cleanup_modue() Richard Guenther: Added support for ISAPnP cards +Paul Gortmaker : Discontinued PCI support - use ne2k-pci.c instead. */ @@ -43,7 +44,6 @@ #include linux/kernel.h #include linux/sched.h #include linux/errno.h -#include linux/pci.h #include linux/isapnp.h #include linux/init.h #include linux/delay.h @@ -75,23 +75,6 @@ }; #endif -#ifdef CONFIG_PCI -/* Ack! People are making PCI ne2000 clones! Oh the horror, the horror... */ -static struct { unsigned short vendor, dev_id; char *name; } -pci_clone_list[] __initdata = { - {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8029, "Realtek 8029" }, - {PCI_VENDOR_ID_WINBOND2,PCI_DEVICE_ID_WINBOND2_89C940, "Winbond 89C940" }, - {PCI_VENDOR_ID_COMPEX, PCI_DEVICE_ID_COMPEX_RL2000, "Compex ReadyLink 2000" }, - {PCI_VENDOR_ID_KTI, PCI_DEVICE_ID_KTI_ET32P2, "KTI ET32P2" }, - {PCI_VENDOR_ID_NETVIN, PCI_DEVICE_ID_NETVIN_NV5000SC, "NetVin NV5000" }, - {PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C926, "VIA 82C926 Amazon" }, - {PCI_VENDOR_ID_SURECOM, PCI_DEVICE_ID_SURECOM_NE34, "SureCom NE34"}, - {0,} -}; - -static int probe_pci = 1; -#endif - static struct { unsigned short vendor, function; char *name; } isapnp_clone_list[] __initdata = { {ISAPNP_VENDOR('E','D','I'), ISAPNP_FUNCTION(0x0216), "NN NE2000" }, @@ -114,7 +97,9 @@ {"ET-100","ET-200", {0x00, 0x45, 0x54}}, /* YANG and YA clone */ {"COMPEX","COMPEX16",{0x00,0x80,0x48}}, /* Broken ISA Compex cards */ {"E-LAN100", "E-LAN200", {0x00, 0x00, 0x5d}}, /* Broken ne1000 clones */ -{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"PCM-4823", "PCM-4823", {0x00, 0xc0, 0x6c}}, /* Broken Advantech MoBo */ +{"REALTEK", "RTL8019", {0x00, 0x00, 0xe8}}, /* no-name with Realtek chip */ +{"LCS-8834", "LCS-8836", {0x04, 0x04, 0x37}}, /* ShinyNet (SET) */ {0,} }; #endif @@ -132,15 +117,9 @@ #define NESM_START_PG 0x40/* First page of TX buffer */ #define NESM_STOP_PG 0x80/* Last page +1 of RX ring */ -/* Non-zero only if the current card is a PCI with BIOS-set IRQ. */ -static unsigned int pci_irq_line = 0; - int ne_probe(struct net_device *dev); static int ne_probe1(struct net_device *dev, int ioaddr); static int ne_probe_isapnp(struct net_device *dev); -#ifdef CONFIG_PCI -static int ne_probe_pci(struct net_device *dev); -#endif static int ne_open(struct net_device *dev); static int ne_close(struct net_device *dev); @@ -180,16 +159,9 @@ {"ne", ne_probe1, NE_IO_EXTENT, netcard_portlist}; #else -/* - * Note that at boot, this probe only picks up one card at a time, even for - * multiple PCI ne2k cards. Use "ether=0,0,eth1" if you have a second PCI - * ne2k card. This keeps things consistent regardless of the bus type of - * the card. - */ - int __init ne_probe(struct net_device *dev) { - int base_addr = dev ? dev-base_addr : 0; + unsigned int base_addr = dev ? dev-base_addr : 0; /* First check any supplied i/o locations. User knows best. cough */ if (base_addr 0x1ff) /* Check a single specified location. */ @@ -197,12 +169,6 @@ else if
Re: [patch] NE2000
Alan Cox wrote: > > > This change sounds ok to me, if noone else objects. (I added to the CC > > a bit) I saw that code, and was thinking about doing the same thing > > myself. ne2k-pci.c definitely has changes which are not included in > > ne.c, and it seems silly to duplicate ne2000 PCI support. > > Unless there are any cards that need the bug workarounds in ne.c for use > on PCI then I see no problem. I've not heard of any. > Ok, I reported it several times, but it gets ignored. I have a Realtek 8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get it stuck by doing a ping -f to a host in the local net, and sometimes it happens doing copies to/from nfs shared resources. rmmod & insmod don't cure the problem, it seems that no interrupts are delivered from the card, and there are no log messages, so a reboot is needed to restore net access. System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit this problem. -- Jorge Nerin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul Gortmaker wrote: > There is no urgency in trying to squeeze a patch like this in the back > door of a 2.4.0 release. For example, there are people out there now > who are using the ne.c driver to run both ISA and PCI cards in the same > box without having to use 2 different drivers. We can wait until 2.5.0 > to break their .config file. IMNSHO this is a bug, though... Do a diff of the key 8390 interface routines in ne.c, and ne2k-pci.c. Ignoring the inb_p and outb_p differences, there are distinct advantages to using ne2k-pci.c on with an NE2000 PCI board. Since ne2k-pci.c supports all boards ne.c does, and includes some fixes that ne.c does not, it seems like removing the PCI support in ne.c is a bug fix change. It looks like ne2k-pci.c does need a HZ scaling fixing from ne.c though... Jeff -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max --- /g/g/tmp/1 Mon Oct 30 14:22:41 2000 +++ /g/g/tmp/2 Mon Oct 30 14:22:58 2000 @@ -1,60 +1,61 @@ /* Hard reset the card. This used to pause for the same period that a 8390 reset command required, but that shouldn't be necessary. */ - -static void ne_reset_8390(struct net_device *dev) +static void +ne2k_pci_reset_8390(struct net_device *dev) { unsigned long reset_start_time = jiffies; - if (ei_debug > 1) - printk(KERN_DEBUG "resetting the 8390 t=%ld...", jiffies); + if (debug > 1) printk("%s: Resetting the 8390 t=%ld...", + dev->name, jiffies); - /* DON'T change these to inb_p/outb_p or reset will fail on clones. */ outb(inb(NE_BASE + NE_RESET), NE_BASE + NE_RESET); ei_status.txing = 0; ei_status.dmaing = 0; /* This check _should_not_ be necessary, omit eventually. */ - while ((inb_p(NE_BASE+EN0_ISR) & ENISR_RESET) == 0) - if (jiffies - reset_start_time > 2*HZ/100) { - printk(KERN_WARNING "%s: ne_reset_8390() did not complete.\n", dev->name); + while ((inb(NE_BASE+EN0_ISR) & ENISR_RESET) == 0) + if (jiffies - reset_start_time > 2) { + printk("%s: ne2k_pci_reset_8390() did not complete.\n", +dev->name); break; } - outb_p(ENISR_RESET, NE_BASE + EN0_ISR); /* Ack intr. */ + outb(ENISR_RESET, NE_BASE + EN0_ISR); /* Ack intr. */ } /* Grab the 8390 specific header. Similar to the block_input routine, but we don't need to be concerned with ring wrap as the header will be at the start of a page, so we optimize accordingly. */ -static void ne_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page) +static void +ne2k_pci_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int +ring_page) { - int nic_base = dev->base_addr; - /* This *shouldn't* happen. If it does, it's the last thing you'll see */ + long nic_base = dev->base_addr; - if (ei_status.dmaing) - { - printk(KERN_EMERG "%s: DMAing conflict in ne_get_8390_hdr " + /* This *shouldn't* happen. If it does, it's the last thing you'll see */ + if (ei_status.dmaing) { + printk("%s: DMAing conflict in ne2k_pci_get_8390_hdr " "[DMAstat:%d][irqlock:%d].\n", dev->name, ei_status.dmaing, ei_status.irqlock); return; } ei_status.dmaing |= 0x01; - outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD); - outb_p(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO); - outb_p(0, nic_base + EN0_RCNTHI); - outb_p(0, nic_base + EN0_RSARLO); /* On page boundary */ - outb_p(ring_page, nic_base + EN0_RSARHI); - outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD); + outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD); + outb(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO); + outb(0, nic_base + EN0_RCNTHI); + outb(0, nic_base + EN0_RSARLO); /* On page boundary */ + outb(ring_page, nic_base + EN0_RSARHI); + outb(E8390_RREAD+E8390_START, nic_base + NE_CMD); - if (ei_status.word16) + if (ei_status.ne2k_flags & ONLY_16BIT_IO) { insw(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)>>1); - else - insb(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)); + } else { + *(u32*)hdr = le32_to_cpu(inl(NE_BASE + NE_DATAPORT)); + le16_to_cpus(>count); + } - outb_p(ENISR_RDC, nic_base + EN0_ISR); /* Ack intr. */ + outb(ENISR_RDC, nic_base + EN0_ISR);/* Ack intr. */ ei_status.dmaing &= ~0x01; } @@ -63,172 +64,116 @@ The NEx000
Re: [patch] NE2000
On Mon, 30 Oct 2000, Paul Gortmaker wrote: > There is no urgency in trying to squeeze a patch like this in the back > door of a 2.4.0 release. For example, there are people out there now > who are using the ne.c driver to run both ISA and PCI cards in the same > box without having to use 2 different drivers. We can wait until 2.5.0 > to break their .config file. I am not quite sure how it will work when you try to use both ne.c and ne2k-pci drivers in the same box. Which driver will be used for PCI card? Maybe people with both cards are forced to use inferior driver for PCI card. Pavel Rabel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Jeff Garzik wrote: > > pavel rabel wrote: > > help. So I removed PCI code from ne.c to have ISA only driver. It > > This change sounds ok to me, if noone else objects. (I added to the CC > a bit) I saw that code, and was thinking about doing the same thing > myself. ne2k-pci.c definitely has changes which are not included in > ne.c, and it seems silly to duplicate ne2000 PCI support. Actually if you look at the archives (ID [EMAIL PROTECTED]) you will see that I've stated this will be done for 2.5 a couple of months ago. (Which is also why PCI support in ne.c hasn't tracked that of ne2k-pci.c - I want to avoid encouraging new PCI users of ne.c) There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. [ I've several other 8390 related patches I've been sitting on - trying to not contribute to the delay of 2.4.0 unless explicitly asked, such as the 8390.h get_module_symbol deletion. Other 8390 patches I have are a separated Tx timeout for 8390.c, kill off dev->rmem_start/end and use ioremap() where required, and replace old check/request_region() with code that makes use of the newer resource structures. ] Good to know people are still keeping an eye out for dead code though... Thanks, Paul. _ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Jeff Garzik wrote: pavel rabel wrote: help. So I removed PCI code from ne.c to have ISA only driver. It This change sounds ok to me, if noone else objects. (I added to the CC a bit) I saw that code, and was thinking about doing the same thing myself. ne2k-pci.c definitely has changes which are not included in ne.c, and it seems silly to duplicate ne2000 PCI support. Actually if you look at the archives (ID [EMAIL PROTECTED]) you will see that I've stated this will be done for 2.5 a couple of months ago. (Which is also why PCI support in ne.c hasn't tracked that of ne2k-pci.c - I want to avoid encouraging new PCI users of ne.c) There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. [ I've several other 8390 related patches I've been sitting on - trying to not contribute to the delay of 2.4.0 unless explicitly asked, such as the 8390.h get_module_symbol deletion. Other 8390 patches I have are a separated Tx timeout for 8390.c, kill off dev-rmem_start/end and use ioremap() where required, and replace old check/request_region() with code that makes use of the newer resource structures. ] Good to know people are still keeping an eye out for dead code though... Thanks, Paul. _ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
On Mon, 30 Oct 2000, Paul Gortmaker wrote: There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. I am not quite sure how it will work when you try to use both ne.c and ne2k-pci drivers in the same box. Which driver will be used for PCI card? Maybe people with both cards are forced to use inferior driver for PCI card. Pavel Rabel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Alan Cox wrote: This change sounds ok to me, if noone else objects. (I added to the CC a bit) I saw that code, and was thinking about doing the same thing myself. ne2k-pci.c definitely has changes which are not included in ne.c, and it seems silly to duplicate ne2000 PCI support. Unless there are any cards that need the bug workarounds in ne.c for use on PCI then I see no problem. I've not heard of any. Ok, I reported it several times, but it gets ignored. I have a Realtek 8029 (ne2k-pci), and with both drivers ne and ne2k-pci I can easily get it stuck by doing a ping -f to a host in the local net, and sometimes it happens doing copies to/from nfs shared resources. rmmod insmod don't cure the problem, it seems that no interrupts are delivered from the card, and there are no log messages, so a reboot is needed to restore net access. System is dual 2x200mmx 96Mb ide discs no interrupts shared, and as far as I can remember all kernel from 2.2.x, 2.3.x up to 2.4.0-testx exhibit this problem. -- Jorge Nerin [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
Paul Gortmaker wrote: There is no urgency in trying to squeeze a patch like this in the back door of a 2.4.0 release. For example, there are people out there now who are using the ne.c driver to run both ISA and PCI cards in the same box without having to use 2 different drivers. We can wait until 2.5.0 to break their .config file. IMNSHO this is a bug, though... Do a diff of the key 8390 interface routines in ne.c, and ne2k-pci.c. Ignoring the inb_p and outb_p differences, there are distinct advantages to using ne2k-pci.c on with an NE2000 PCI board. Since ne2k-pci.c supports all boards ne.c does, and includes some fixes that ne.c does not, it seems like removing the PCI support in ne.c is a bug fix change. It looks like ne2k-pci.c does need a HZ scaling fixing from ne.c though... Jeff -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max --- /g/g/tmp/1 Mon Oct 30 14:22:41 2000 +++ /g/g/tmp/2 Mon Oct 30 14:22:58 2000 @@ -1,60 +1,61 @@ /* Hard reset the card. This used to pause for the same period that a 8390 reset command required, but that shouldn't be necessary. */ - -static void ne_reset_8390(struct net_device *dev) +static void +ne2k_pci_reset_8390(struct net_device *dev) { unsigned long reset_start_time = jiffies; - if (ei_debug 1) - printk(KERN_DEBUG "resetting the 8390 t=%ld...", jiffies); + if (debug 1) printk("%s: Resetting the 8390 t=%ld...", + dev-name, jiffies); - /* DON'T change these to inb_p/outb_p or reset will fail on clones. */ outb(inb(NE_BASE + NE_RESET), NE_BASE + NE_RESET); ei_status.txing = 0; ei_status.dmaing = 0; /* This check _should_not_ be necessary, omit eventually. */ - while ((inb_p(NE_BASE+EN0_ISR) ENISR_RESET) == 0) - if (jiffies - reset_start_time 2*HZ/100) { - printk(KERN_WARNING "%s: ne_reset_8390() did not complete.\n", dev-name); + while ((inb(NE_BASE+EN0_ISR) ENISR_RESET) == 0) + if (jiffies - reset_start_time 2) { + printk("%s: ne2k_pci_reset_8390() did not complete.\n", +dev-name); break; } - outb_p(ENISR_RESET, NE_BASE + EN0_ISR); /* Ack intr. */ + outb(ENISR_RESET, NE_BASE + EN0_ISR); /* Ack intr. */ } /* Grab the 8390 specific header. Similar to the block_input routine, but we don't need to be concerned with ring wrap as the header will be at the start of a page, so we optimize accordingly. */ -static void ne_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int ring_page) +static void +ne2k_pci_get_8390_hdr(struct net_device *dev, struct e8390_pkt_hdr *hdr, int +ring_page) { - int nic_base = dev-base_addr; - /* This *shouldn't* happen. If it does, it's the last thing you'll see */ + long nic_base = dev-base_addr; - if (ei_status.dmaing) - { - printk(KERN_EMERG "%s: DMAing conflict in ne_get_8390_hdr " + /* This *shouldn't* happen. If it does, it's the last thing you'll see */ + if (ei_status.dmaing) { + printk("%s: DMAing conflict in ne2k_pci_get_8390_hdr " "[DMAstat:%d][irqlock:%d].\n", dev-name, ei_status.dmaing, ei_status.irqlock); return; } ei_status.dmaing |= 0x01; - outb_p(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD); - outb_p(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO); - outb_p(0, nic_base + EN0_RCNTHI); - outb_p(0, nic_base + EN0_RSARLO); /* On page boundary */ - outb_p(ring_page, nic_base + EN0_RSARHI); - outb_p(E8390_RREAD+E8390_START, nic_base + NE_CMD); + outb(E8390_NODMA+E8390_PAGE0+E8390_START, nic_base+ NE_CMD); + outb(sizeof(struct e8390_pkt_hdr), nic_base + EN0_RCNTLO); + outb(0, nic_base + EN0_RCNTHI); + outb(0, nic_base + EN0_RSARLO); /* On page boundary */ + outb(ring_page, nic_base + EN0_RSARHI); + outb(E8390_RREAD+E8390_START, nic_base + NE_CMD); - if (ei_status.word16) + if (ei_status.ne2k_flags ONLY_16BIT_IO) { insw(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)1); - else - insb(NE_BASE + NE_DATAPORT, hdr, sizeof(struct e8390_pkt_hdr)); + } else { + *(u32*)hdr = le32_to_cpu(inl(NE_BASE + NE_DATAPORT)); + le16_to_cpus(hdr-count); + } - outb_p(ENISR_RDC, nic_base + EN0_ISR); /* Ack intr. */ + outb(ENISR_RDC, nic_base + EN0_ISR);/* Ack intr. */ ei_status.dmaing = ~0x01; } @@ -63,172 +64,116 @@ The NEx000 doesn't share the
Re: [patch] NE2000
> This change sounds ok to me, if noone else objects. (I added to the CC > a bit) I saw that code, and was thinking about doing the same thing > myself. ne2k-pci.c definitely has changes which are not included in > ne.c, and it seems silly to duplicate ne2000 PCI support. Unless there are any cards that need the bug workarounds in ne.c for use on PCI then I see no problem. I've not heard of any. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
pavel rabel wrote: > There are three drivers for n2k cards. One is MCA only, one is PCI only, > and the then the third one (ne.c) is both ISA and PCI. I think the ISA > driver should be ISA only, as is described in Documentation and in config > help. So I removed PCI code from ne.c to have ISA only driver. It > gets a bit smaller, although I am not sure whether more code can be > removed. This change sounds ok to me, if noone else objects. (I added to the CC a bit) I saw that code, and was thinking about doing the same thing myself. ne2k-pci.c definitely has changes which are not included in ne.c, and it seems silly to duplicate ne2000 PCI support. Regards, Jeff P.S. Pavel, for the future, patches made with "diff -u" are preferred. -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
pavel rabel wrote: There are three drivers for n2k cards. One is MCA only, one is PCI only, and the then the third one (ne.c) is both ISA and PCI. I think the ISA driver should be ISA only, as is described in Documentation and in config help. So I removed PCI code from ne.c to have ISA only driver. It gets a bit smaller, although I am not sure whether more code can be removed. This change sounds ok to me, if noone else objects. (I added to the CC a bit) I saw that code, and was thinking about doing the same thing myself. ne2k-pci.c definitely has changes which are not included in ne.c, and it seems silly to duplicate ne2000 PCI support. Regards, Jeff P.S. Pavel, for the future, patches made with "diff -u" are preferred. -- Jeff Garzik | "Mind if I drive?" -Sam Building 1024 | "Not if you don't mind me clawing at the MandrakeSoft| dash and shrieking like a cheerleader." | -Max - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] NE2000
This change sounds ok to me, if noone else objects. (I added to the CC a bit) I saw that code, and was thinking about doing the same thing myself. ne2k-pci.c definitely has changes which are not included in ne.c, and it seems silly to duplicate ne2000 PCI support. Unless there are any cards that need the bug workarounds in ne.c for use on PCI then I see no problem. I've not heard of any. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/