First, a question especially to Auke and Jeff:
Since this patch both reverts the broken change that is currently in
-rc and creates the fixed driver, I'm not sure I like the subject
stating "on ARM", although that is the feature of the rewrite, and was
the intent of merging the previous patch. This is actually its a fix
for all systems relative to current, including those where dma is not
cache coherent, (unlike a simple revert).
Should we just put a comment about reverting the previous patch early
in the change log?
Something like this:
Fix the e100 receiver handling, supporting cache incoherent DMA.
Discard the concept of setting the S (suspend) bit with the EL bit
introduced in commit d52df4a35af569071fda3f4eb08e47cc7023f094. In
addition to it not setting either bit, the hardware doesn't work that
way.
Thoughts?
Here is the changelog portion of the latest patch (quoted), with my
comments thrown in:
On the ARM, their is a race condition between software allocating a
new receive
On systems that have cache incoherent DMA, including ARM,
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.
This can lead to the receive unit stalling or interpreting random
memory as its receive area.
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.
Software can write to the tail of the list because it knows hardware
will stop on the previous descriptor that was marked as the end of
list.
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.
and we can handle the race setting the size (assuming aligned 16 bit
writes are atomic with respect to the DMA read).
This paragraph changed from third person (the software or hardware) to
second person (we).
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.
This way software can identify them when the race may have occurred
when cleaning the ring. On these descriptors, it looks ahead and if
the next one is complete then hardware must have skipped the current
one. Logic is added to prevent two packets in a row being marked while
the receiver is running to avoid running in lockstep with the hardware
and thereby limiting the required lookahead.
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.
These sentences should be moved to the mention of the race above to
reducing mixing descriptions of the hardware and the software.
milton
-
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
