David Acker wrote:
Milton Miller wrote:
Your logic works, this adds some conditional branching but saves a
pointer, not sure which is better. Mine can be used for initializing
without special casing since it will just calculate rx_to_unmark as
rx[n-2] and rx_to_mark as rx[n-2] != rx[n-2]->prev; unmarking a never
marked still works, where as for yours the "was marked" must be
explicitly set to something (hmm, rxs = rx[0] might work for that
initial value until we mark -2??)
again, the compare of rx->el instead of rx->rfd->el is to save cache
traffic to the rfd under io. The rx->was_0 is required, so the el
flag is free.
Ok, I took a stab at coding and testing these ideas. Below is a patch
against 2.6.22-rc3.
Let me know what you think. Testing shows that we can hit any of the
following scenarios:
Find a buffer not complete with rx->el and rx->s0 set. I read back the
status and see if
the receiver is still running.
Find a buffer not complete with rx->el not set and rx->s0 set. I check
the next buffer
and if it is complete, I free the skb and return 0. If the next buffer
is not
complete, I read the receiver's status to see if he is still running.
Find a buffer that is complete with rx->el not set and rx->s0 set. It
appears that hardware
can read the rfd's el-bit, then software can clear the rfd el-bit and
set the rfd size, and
then hardware can come in and read the size. I am reading the status
back, although
I don't think that I have to in this instance.
I am testing a version of this code patched against 2.6.18.4 on my PXA
255 based system. I will let you all know how it goes.
On the ARM, their is a race condition between software allocating a new
receive
buffer and hardware writing into a buffer. The two race on touching the
last
Receive Frame Descriptor (RFD). It has its el-bit set and its next link
equal
to 0. When hardware encounters this buffer it attempts to write data to it
and then update Status Word bits and Actual Count in the RFD. At the
same time
software may try to clear the el-bit and set the link address to a new
buffer.
Since the entire RFD is once cache-line, the two write operations can
collide. This can lead to the receive unit stalling or freed receive
buffers
getting written to.
The fix is to set the el-bit on and the size to 0 on the next to last
buffer
in the chain. When the hardware encounters this buffer it stops and does
not write to it at all. The hardware issues an RNR interrupt with the
receive unit in the No Resources state. When software allocates buffers,
it can update the tail of the list when either it knows the hardware
has stopped or the previous to the new one to mark marked.
Once it has a new next to last buffer prepared, it can clear the el-bit
and set the size on the previous one. The race on this buffer is safe
since the link already points to a valid next buffer. We keep flags
in our software descriptor to note if the el bit is set and if the size
was 0. When we clear the RFD's el bit and set its size, we also clear
the el flag but we leave the size was 0 bit set. This was we can find
this buffer again later.
If the hardware sees the el-bit cleared without the size set, it will
move on to the next buffer and skip this one. If it sees
the size set but the el-bit still set, it will complete that buffer
and then RNR interrupt and wait.
Signed-off-by: David Acker <[EMAIL PROTECTED]>
That seems to vaguely match my memory of what eepro100 was doing (or
trying to do).
I _really_ appreciate you working on this problem. Getting e100 driver
stable for the long term, and ditching eepro100, is a big hurdle to
cross. Getting this right is really one of the last steps.
The patch looks OK at quick glance.
Thanks,
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
