[PATCH 18/45] score: Add Thread_queue_Control::Lock

[Gedare Bloom]

On Mon, May 18, 2015 at 3:20 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
>
> On 17/05/15 14:14, Gedare Bloom wrote:
>>
>> On Sun, May 17, 2015 at 7:04 AM, Gedare Bloom <gedare at rtems.org> wrote:
>>>
>>> On Fri, May 15, 2015 at 7:41 AM, Sebastian Huber
>>> <sebastian.huber at embedded-brains.de> wrote:
>>>>
>>>>   /**
>>>> - *  @brief Gets a pointer to the "first" thread on the_thread_queue.
>>>> + * @brief Returns the first thread on the thread queue if it exists,
>>>> otherwise
>>>> + * @c NULL (locked).
>>>> + *
>>>> + * The caller must be the owner of the thread queue lock.
>>>> + *
>>>> + * @param[in] the_thread_queue The thread queue.
>>>>    *
>>>> - *  This function returns a pointer to the "first" thread
>>>> - *  on the_thread_queue.  The "first" thread is selected
>>>> - *  based on the discipline of the_thread_queue.
>>>> + * @retval NULL No thread is present on the thread queue.
>>>> + * @retval first The first thread on the thread queue according to the
>>>> enqueue
>>>> + * order.
>>>> + */
>>>> +Thread_Control *_Thread_queue_First_locked(
>>>> +  Thread_queue_Control *the_thread_queue
>>>> +);
>>>> +
>>>
>>> In other places we call such a function variant _unprotected. In the
>>> doxygen, I don't know what you mean by @c NULL (locked) in the @brief,
>>> or why there are two @brief.
>>
>> Please clarify the distinctions between _locked(), _critical(), and
>> _unprotected().
>
>
> The _locked() is commonly used in Linux and FreeBSD and indicates that the
> caller must be the lock owner. The _unprotected() is mainly used for the
> chains.  Once the Giant lock is eliminated, we can delete the "protected"
> chain variants and drop the _unprotected().
>
OK.

> What about @c NULL (locked) -> @c NULL (caller locked variant)?
>
Well I don't see a need for anything in parens. I first read the
(locked) to imply this was a condition. I would instead prefer a more
explicit comment in the @brief that says the caller must hold the
lock. Eventually  of course we can assume this is the case for
functions named _locked(*).

> The _critical() functions call _Thread_Dispatch_disable_critical() and do an
> ISR lock to thread dispatch disabled transition to carry out scheduler
> operations.
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax     : +49 89 189 47 41-09
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>