On Fri, Jun 12, 2015 at 08:55:09PM +0100, Dave Gordon wrote: > > However, that makes an incorrect assumption about the waiter. Given that > > the current code is written such that ringbuf->last_retired_head = > > request->postfix and that space is identical to the repeated > > calculation, what is your intention exactly? > > -Chris > > Three factors: > > * firstly, a preference: I find it logically simpler that there should > be one and only one piece of code that writes into ringbuf->space. One > doesn't then have to reason about whether two different calculations are > in fact equivalent. I find the opposite, since we compute how much space we want I think it is counter intuitive to suggest otherwise. You then need to assert that the computed space is enough. By saying if we wait until this request, we must have at least this space available and only using that space there is no implicit knowlege. > * secondly, for future proofing: although wait_request() now retires > only up to the waited-for request, that wasn't always the case, nor is > there any reason why it could not in future opportunistically retire > additional requests that have completed while it was waiting. Because there is a cost associated with every retirement. See above for why being explicit is clearer. > * thirdly, for correctness: using the function has the additional effect > of consuming the last_retired_head value set by retire_request. If this > is not done, a later call to intel_ring_space() may become confused, > with the result that 'head' (and therefore 'space') will be incorrectly > updated. Eh. The code is still strictly correct. The biggest issue is that we have multiple locations that decide how to interpret the amount of space generated by completing the request. However, we want to keep request retirement very simple since it is a hot function. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
