Hi, Peierre and Keyon, Thanks for your comments. Because size = min(hsize, lsize), the updated host_offset should not > host_size. So "==" is enough. I will submit it again after modification. Thanks again.
Yan Wang On Thu, 2017-11-09 at 20:31 -0600, Pierre-Louis Bossart wrote:
@@ -90,7 +90,7 @@ static uint64_t trace_work(void *data, uint64_t delay) /* update host pointer and check for wrap */ d->host_offset += size; - if (d->host_offset + size >= d->host_size) + if (d->host_offset >= d->host_size) d->host_offset = 0;
this code pattern is present multiple times in the DMA handling code. I wonder if this is correct when the > condition is true? That'd mean some overrun happened or that some bytes have been lost. Shouldn't this be an exact == condition in there is a true wrap?
actually we should make it to something like this for sake of safe: d->host_offset -= d->host_size;
but considering that we have require the host_size to be multiple of period size, the '>' case won't happen, so here it is simplified to be d->host_offset = 0;
I agree that we should make it more robust.
Yes, my point exactly. Either the > condition happens and we have to handle it, or it doesn't by construction due to other restrictions and the test can be simplified.