> From: "Bill Fischofer" <[email protected]> > To: "Nicolas Morey-Chaisemartin" <[email protected]> > Cc: "LNG ODP Mailman List" <[email protected]>, "Maxim Uvarov" > <[email protected]> > Sent: Thursday, 3 September, 2015 6:19:52 PM > Subject: Re: [lng-odp] [PATCHv2 6/8] linux-generic: queue: correct handling > of reorder completion
> On Thu, Sep 3, 2015 at 11:05 AM, Nicolas Morey-Chaisemartin < > [email protected] > wrote: > > On 09/03/2015 05:16 PM, Bill Fischofer wrote: > > > > @@ -910,23 +897,28 @@ int release_order(queue_entry_t *origin_qe, > > > uint64_t > > > order, > > > > order_release(origin_qe, 1); > > > > > > > > /* Check if this release allows us to unblock waiters. > > > > - * Note that we can only process complete waiters since > > > > - * if the sustain bit is set for a buffer this means that > > > > - * some other thread is working on it and will be > > > > - * responsible for resolving order when it is complete. > > > > + * At the point of this call, the reorder list may contain > > > > + * zero or more placeholders that need to be freed, followed > > > > + * by zero or one complete reorder buffer chain. > > > > */ > > Note that this comment was simplified in v2 of the patch > > I'm struggling to see why this is true. > > > From my understanding, the reorder list is sorted solely on order. > > > Place holders are inserted when a thread releases a scheduler while it's > > not > > its turn yet (thread order > order_out). > > > So let's assume 5 threads pulling each 1 buffer ordered 0 to 4. > > > For some reason, T0 is busy and not releasing/Sending anything so order_out > > stays stuck at 0 > > > T1 frees the buffer, releases the scheduler => a place holder is inserted > > (ReorderList = P1) > > > T2 sends the buffer => Reorder list = P1 B2 > > > Scheduler release clears the sustain bit here > > > T3 frees the buffer and releases > > > T4 sends the buffer > > > In the end the reorder list should look like P1 B2 P3 B4 > > > Or I am missing something here. > > That's correct. The corner case we're avoiding here is let's say T2 has sent > one or more buffers but hasn't released order yet. So the Reorder list may > look like: > P1, B2a, B2b, P3, B4 > Now suppose T0 finishes and so it resolves its order. As part of that order > processing we'll bump order_out and see that we can now also remove P1, > which bumps the order again. It's now the case that T3 is in order, but the > two buffers on the list have their sustain bits set, which means that T3 is > still running and could issue more enqueus. If we tried to process B2a and > B2b here, we'd be in a race condition between two threads trying to process > the same order, and this would likely result in order inversion. > So the rule is you can only process buffers on the list if the buffer(s) for > that order end with a buffer that has sustain=0, because that means that the > thread that held that order has released it's context so there can be no > race condition. > Make sense? It does. I'm not at work anymore so I can't check the code but from what I saw release_order is only called when the scheduler is released. And from what I see in this patch, we never pull more than 1 EVent (or maybe a list of event if they have the same order) at once. So in my previous case, when T0 releases the scheduler, we free P1, send B2 and then stop (or maybe also 3 P3, not sure about that one) but B4 stays in the reorder queue although it could be sent ! Assuming that no other packet comes and all the queues are empty, how is B4 sent ? Nicolas
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
