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

Reply via email to