Petri Savolainen(psavol) replied on github web page: include/odp/api/spec/schedule.h line 43 @@ -365,7 +368,28 @@ void odp_schedule_order_lock(unsigned lock_index); * hold this lock. Must be in the range * 0..odp_queue_lock_count() - 1 */ -void odp_schedule_order_unlock(unsigned lock_index); +void odp_schedule_order_unlock(uint32_t lock_index); + +/** + * Release existing ordered context lock and acquire a new lock + * + * This call is valid only when holding an ordered synchronization context. + * Release a previously locked ordered context lock and acquire a new ordered + * context lock. The operation is equivalent to application calling first + * odp_schedule_order_unlock(unlock_index) and then + * odp_schedule_order_lock(lock_index). The same constraints apply to this call + * similar to calling these two calls in sequence. + * + * @param unlock_index Index of the acquired ordered lock in the current + * context to be released. + * @param lock_index Index of the ordered lock in the current context to be + * acquired. Must be in the range + * 0...odp_queue_lock_count() - 1. + * + * @see odp_schedule_order_lock(), odp_schedule_order_unlock() + * + */ +void odp_schedule_order_unlock_lock(uint32_t unlock_index, uint32_t lock_index);
Comment: Maybe this "The same constraints apply to this call similar to calling these two calls in sequence" needs to be re-worded to be more fluent. E.g. "The same constraints apply with this call as with those two." > Petri Savolainen(psavol) wrote: > "Results are undefined if the > ordered lock with the given unlock_index is not held by > this ordered context." > > Duplication on unlock() API text. Better to remove this from here. > >> Petri Savolainen(psavol) wrote: >> It's better to remove this duplicate spec ... >> "This call is valid only when there is a single ordered context >> lock active. Results are undefined if this call is made when multiple ordered >> context locks are acquired in nested fashion. >> This call does not release the existing ordered context and only switches >> from one ordered lock to another ordered lock within the same ordered >> context." >> >> ... and refer to lock()/unlock() spec instead: "The operation is equivalent >> to application calling first odp_schedule_order_unlock(unlock_index) and >> then odp_schedule_order_lock(lock_index). The same constraints apply to this >> call than those two." >>> Petri Savolainen(psavol) wrote: >>> Remove this outdated sentence: " Previously acquired ordered lock is >>> released." We have now unlock_index to identify which lock is unlocked. >>>> muvarov wrote >>>> @psavol only comment like you did. >>>>> Petri Savolainen(psavol) wrote: >>>>> Was there some benefit of not having index of the unlocked lock as a >>>>> parameter ? Can't remember if I suggested it myself. Anyway, >>>>> implementation would simplify and it would still be as robust as unlock() >>>>> (which application may pass bad lock index). >>>>>> Petri Savolainen(psavol) wrote: >>>>>> Don't know how to comment on COMMIT LOG on Github ! >>>>>> >>>>>> Anyway, commit log needs correct format "linux-gen: sched: ...." and >>>>>> some more text (it's empty now) >>>>>>> Petri Savolainen(psavol) wrote: >>>>>>> "This call does not release the existing ordered context and only >>>>>>> switches >>>>>>> from one ordered lock to another ordered lock within the same ordered >>>>>>> context." >>>>>>> >>>>>>> I'd remove this since this call is not different to lock/unlock calls >>>>>>> in this regard >>>>>>>> Petri Savolainen(psavol) wrote: >>>>>>>> "This call is valid only when there is a single ordered context >>>>>>>> lock active. Results are undefined if this call is made when multiple >>>>>>>> ordered >>>>>>>> context locks are acquired in nested fashion. " >>>>>>>> >>>>>>>> Why nested locks are banned? I think it should not make a difference. >>>>>>>> Unlock_lock(C) is equivalent to unlock(B) + lock(C) under. >>>>>>>> >>>>>>>> lock(A) >>>>>>>> lock(B) >>>>>>>> unlock(B) >>>>>>>> lock(C) >>>>>>>> ... >>>>>>>>> Petri Savolainen(psavol) wrote: >>>>>>>>> To match other function prototypes, should be **unsigned** instead of >>>>>>>>> uint32_t. >>>>>>>>>> Petri Savolainen(psavol) wrote: >>>>>>>>>> It implements ordered lock API by handling all queues as atomic. >>>>>>>>>> >>>>>>>>>> It demonstrates a very simple implementation of the API, and thus is >>>>>>>>>> an easy starting for new scheduler development. >>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>> I added this change for scenario where >>>>>>>>>>> odp_schedule_order_unlock_lock() is called after a lock has been >>>>>>>>>>> released using odp_schedule_order_unlock() but since we are >>>>>>>>>>> defining in the documentation this is an undefined behaviour I >>>>>>>>>>> believe your suggestion makes sense. >>>>>>>>>>> I will update in the next version. >>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>>> Done >>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>>>> I will be moving lock_index from rctx to >>>>>>>>>>>>> sched_scalable_thread_state_t in the next version and thanks for >>>>>>>>>>>>> pointing it out. >>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>>>>> The initial draft of ordered lock considered API for locking and >>>>>>>>>>>>>> unlocking ordered locks within a context. We found an use-case >>>>>>>>>>>>>> from the customer where the application was splitting a single >>>>>>>>>>>>>> ordered flow to two different flows, which is possible with our >>>>>>>>>>>>>> current API architecture but the drawback is that it requires >>>>>>>>>>>>>> the application to make two different calls >>>>>>>>>>>>>> odp_schedule_unlock() and odp_schedule_lock() which will have >>>>>>>>>>>>>> performance issues since the implementation has to internally >>>>>>>>>>>>>> switch out of the lock and then acquire the new lock. The main >>>>>>>>>>>>>> objective of this new API is to optimize this use-case by using >>>>>>>>>>>>>> a single API which switches the ordered context from one ordered >>>>>>>>>>>>>> lock to another. >>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>>>>>> Not true. Linux-generic implementation is using an uint16_t but >>>>>>>>>>>>>>> as explained before implementations are free to use any number >>>>>>>>>>>>>>> of bits as required. The number of bits of lock_index also >>>>>>>>>>>>>>> depends upon the application use-case. >>>>>>>>>>>>>>> Ordered locks is not related to number of threads in the system >>>>>>>>>>>>>>> but to the total number of flows in the system. Depending upon >>>>>>>>>>>>>>> the implementation it could support a huge number of flows ( >>>>>>>>>>>>>>> 2^20 ) and not all flows will be active. >>>>>>>>>>>>>>>> Balasubramanian Manoharan(bala-manoharan) wrote: >>>>>>>>>>>>>>>> Just to reiterate, ordered locks are used to make sure packet >>>>>>>>>>>>>>>> belonging to a particular flow acquire the lock in order. >>>>>>>>>>>>>>>> There is no need to acquire the lock in ascending order. Even >>>>>>>>>>>>>>>> though our linux-generic implementation is using them as index >>>>>>>>>>>>>>>> number for performance reasons it will usually be the last few >>>>>>>>>>>>>>>> bits of the flow id calculated by RSS or any other mechanism >>>>>>>>>>>>>>>> decided by the application. >>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>> As noted above, since `odp_schedule_order_unlock_lock()` only >>>>>>>>>>>>>>>>> takes one parameter (the index of the new lock to be >>>>>>>>>>>>>>>>> acquired), we need a place to store the index of the lock >>>>>>>>>>>>>>>>> that the caller now holds to be used as an implied parameter >>>>>>>>>>>>>>>>> to that API. So this is supposed to be a thread-local >>>>>>>>>>>>>>>>> variable. If that's not what's happening here then that needs >>>>>>>>>>>>>>>>> to be fixed, so thanks for pointing that out. >>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>> The lock_index is an implied parameter to >>>>>>>>>>>>>>>>>> `odp_schedule_order_unlock_lock()`. It's the index of the >>>>>>>>>>>>>>>>>> lock that the caller currently holds, so this is a >>>>>>>>>>>>>>>>>> thread-local variable. It's an error to call this API >>>>>>>>>>>>>>>>>> without already holding an ordered lock. >>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>> ODP does not have any architectural requirement that >>>>>>>>>>>>>>>>>>> ordered locks be acquired in any particular sequence, or >>>>>>>>>>>>>>>>>>> that they be acquired at all. The only requirement is that >>>>>>>>>>>>>>>>>>> they only can be acquired once per ordered context. So this >>>>>>>>>>>>>>>>>>> wording is correct. It's entirely possible that a thread >>>>>>>>>>>>>>>>>>> acquires lock 1 first and then switches to lock 0. It's >>>>>>>>>>>>>>>>>>> entirely up to the application. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> The beauty of ordered locks is that they are deadlock-free >>>>>>>>>>>>>>>>>>> by design even in the face of random acquisition order >>>>>>>>>>>>>>>>>>> among the various threads that use them. >>>>>>>>>>>>>>>>>>>> WonderfulVoid wrote >>>>>>>>>>>>>>>>>>>> @psavol The SP scheduler only exists to provide strict >>>>>>>>>>>>>>>>>>>> priority scheduling? The scalable scheduler does that as >>>>>>>>>>>>>>>>>>>> well (and doesn't the default scheduler also do basic SP >>>>>>>>>>>>>>>>>>>> scheduling?). Are there any specific characteristics of >>>>>>>>>>>>>>>>>>>> the SP scheduler (beyond the basic SP scheduling) that are >>>>>>>>>>>>>>>>>>>> of interest? In benchmarking, the SP scheduler seemed to >>>>>>>>>>>>>>>>>>>> be the slowest by far. >>>>>>>>>>>>>>>>>>>>> WonderfulVoid wrote >>>>>>>>>>>>>>>>>>>>> The reorder window (rwin) is shared by all threads, you >>>>>>>>>>>>>>>>>>>>> can't store per-thread data in it, you would use the >>>>>>>>>>>>>>>>>>>>> reorder context (rctx) for that. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> But why are we saving the lock_index? Is it used for >>>>>>>>>>>>>>>>>>>>> making any correctness checks (e.g. verifying that the >>>>>>>>>>>>>>>>>>>>> application isn't unlocking some lock it hasn't acquired)? >>>>>>>>>>>>>>>>>>>>>> WonderfulVoid wrote >>>>>>>>>>>>>>>>>>>>>> Why is this uint32_t? The lock_count (defined below) is >>>>>>>>>>>>>>>>>>>>>> only an uint16_t. We don't support more than 65535 locks >>>>>>>>>>>>>>>>>>>>>> (per ordered queue). The ordered locks themselves use >>>>>>>>>>>>>>>>>>>>>> uint32_t (for the "ticket") but also that is overkill, >>>>>>>>>>>>>>>>>>>>>> there just won't be that many threads. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Also this seems to be used for per-thread purposes >>>>>>>>>>>>>>>>>>>>>> (schedule_order_lock() saves the passed lock_index here) >>>>>>>>>>>>>>>>>>>>>> but the reorder_window is shared by all threads and >>>>>>>>>>>>>>>>>>>>>> cannot be used for per-thread data. If lock_index needs >>>>>>>>>>>>>>>>>>>>>> to be saved (is it actually used for anything?), it >>>>>>>>>>>>>>>>>>>>>> should be saved in the reorder_context. >>>>>>>>>>>>>>>>>>>>>>> WonderfulVoid wrote >>>>>>>>>>>>>>>>>>>>>>> Shouldn't lock_index be in the range >>>>>>>>>>>>>>>>>>>>>>> 1..odp_queue_lock_count()-1? We are supposedly already >>>>>>>>>>>>>>>>>>>>>>> holding one lock (lock_index >=0), this lock will be >>>>>>>>>>>>>>>>>>>>>>> released and the next lock will be acquired. >>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>> Delete, as per previous explanation. The scalable >>>>>>>>>>>>>>>>>>>>>>>> scheduler could be further optimized by changing its >>>>>>>>>>>>>>>>>>>>>>>> runtime checks to ODP_ASSERT()s, but that's beyond the >>>>>>>>>>>>>>>>>>>>>>>> scope of this PR. >>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>> This needs to go after the `if` statement, not inside >>>>>>>>>>>>>>>>>>>>>>>>> it, since the whole statement is skipped if the lock >>>>>>>>>>>>>>>>>>>>>>>>> is immediately available--it's only there to wait for >>>>>>>>>>>>>>>>>>>>>>>>> the lock. >>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> Delete, as per above explanation. >>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> Again, this should have a doxygen comment for >>>>>>>>>>>>>>>>>>>>>>>>>>> consistency. >>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> Nice way to avoid the extraneous checking in >>>>>>>>>>>>>>>>>>>>>>>>>>>> `schedule_order_unlock_lock()`, but actually this >>>>>>>>>>>>>>>>>>>>>>>>>>>> isn't needed at all due to the prior >>>>>>>>>>>>>>>>>>>>>>>>>>>> `ODP_ASSERT()`. That assert requires that the >>>>>>>>>>>>>>>>>>>>>>>>>>>> `lock_index` lock be held at entry to >>>>>>>>>>>>>>>>>>>>>>>>>>>> `schedule_order_unlock` so if >>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_schedule_order_unlock_unlock()` is called >>>>>>>>>>>>>>>>>>>>>>>>>>>> without an ordered lock being held, the previous >>>>>>>>>>>>>>>>>>>>>>>>>>>> `ODP_ASSERT()` will detect that and abort. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> Interestingly, this also means that (in odp-linux, >>>>>>>>>>>>>>>>>>>>>>>>>>>> at least) you can call >>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_schedule_order_unlock_lock()` while holding >>>>>>>>>>>>>>>>>>>>>>>>>>>> nested ordered locks. The semantics is always to >>>>>>>>>>>>>>>>>>>>>>>>>>>> release the most recently acquired ordered lock >>>>>>>>>>>>>>>>>>>>>>>>>>>> and replace that with a new ordered lock >>>>>>>>>>>>>>>>>>>>>>>>>>>> regardless of how many other ordered locks the >>>>>>>>>>>>>>>>>>>>>>>>>>>> caller holds in this ordered context. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> So the net is you can delete this statement >>>>>>>>>>>>>>>>>>>>>>>>>>>> without any loss of correctness. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> This should have a doxygen description for >>>>>>>>>>>>>>>>>>>>>>>>>>>>> consistency with surrounding vars. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I didn't realize the schedule_sp didn't >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> implement ordered locks. @psavol is this a "to >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> do" item? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> A simple reference to the stored `lock_index` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> is sufficient here. No need for atomics. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This can be optimized into an `ODP_ASSERT()` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to be consistent with the other schedulers. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Behavior is undefined if these routines are >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not called within an ordered context, so no >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> need to take a runtime hit to do these type of >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> checks. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Same comment here. A simple `lock_index` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tracker is all that's needed. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Again, no need for fancy atomics here since >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you're already protected by the ordered lock >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> itself. You're just remembering the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `lock_index` you locked. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I suspect same comments for this patch >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> apply equally to all of the scheduler >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> variants. Not repeating individual review >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> comments here. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Replace above two lines with: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ``` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> schedule_order_unlock(sched_local.ordered.lock_index); >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ``` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Add here: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ``` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ODP_ASSERT(queue_index != NULL_INDEX && >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> lock_index <= >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sched->queue[queue_index].order_lock_count >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> && >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> !sched_local.ordered.lock_called.u8[lock_index] >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> && >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sched_local.ordered.lock_called.u8[sched_local.ordered.lock_index]); >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ``` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unnecessary >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unnecessary >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unnecesssary >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> All that's needed here is a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `sched_local.ordered.lock_index = >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> lock_index;` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Not the best choice of name. You're >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not tracking that a lock has been >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> acquired, but rather the index value >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the lock that has been acquired. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unnecessary. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unnecessary >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Why does this need to be an >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_atomic_u64_t`? All you're >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> doing is tracking the index of >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the currently owned ordered lock >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> so you can use this as an >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> implicit parameter to >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_schedule_order_unlock_lock()`. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> As such a `uint16_t` (or even a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `uint8_t`) should be sufficient >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> here. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> But this information is part of >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the `sched_local.ordered` struct. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You might wish to clarify that >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> this call does not release the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ordered context that contains >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the lock being unlocked. This >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> call simply changes from one >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ordered lock to another within >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the current ordered context. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Typo: acquire >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fischofer(Bill-Fischofer-Linaro) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Typo: acquire https://github.com/Linaro/odp/pull/160#discussion_r138846016 updated_at 2017-09-14 09:47:08