[PATCH 2/5] drm: Add Reusable task barrier.

Andrey Grodzovsky Andrey.Grodzovsky at amd.com
Thu Dec 12 15:10:55 UTC 2019 

On 12/12/19 10:09 AM, Christian König wrote:
> Am 12.12.19 um 15:50 schrieb Grodzovsky, Andrey:
>> [AMD Official Use Only - Internal Distribution Only]
>>
>> ______________________________________
>> From: Christian König <ckoenig.leichtzumerken at gmail.com>
>> Sent: 12 December 2019 03:31
>> To: Alex Deucher; Grodzovsky, Andrey
>> Cc: Deucher, Alexander; Ma, Le; Quan, Evan; amd-gfx list; Zhang, Hawking
>> Subject: Re: [PATCH 2/5] drm: Add Reusable task barrier.
>>
>> Am 12.12.19 um 09:24 schrieb Christian König:
>>> Am 11.12.19 um 21:19 schrieb Alex Deucher:
>>>> On Wed, Dec 11, 2019 at 3:07 PM Andrey Grodzovsky
>>>> <andrey.grodzovsky at amd.com> wrote:
>>>>> It is used to synchronize N threads at a rendevouz point before
>>>>> execution
>>>>> of critical code that has to be started by all the threads at
>>>>> approximatly
>>>>> the same time.
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>>>> You should resend to dri-devel since this task barrier is being added
>>>> to common code.
>>> Additional to that this whole thing has the potential to raise lockdep
>>> warnings and if I'm not completely mistaken doesn't even work 
>>> correctly.
>> Can you give me a potential lockdep scenario ?
>
> Lockdep usually complains if a lock is released from another thread 
> than where it was locked from.
>
> In the code you let each thread do a down() and then the last one does 
> multiple up() calls.
>
> But I think that is only illegal for mutexes, but legal for semaphores.
>
> Christian.


Yes, from what I've read for semaphores it's ok to release (up) from a 
thread which didn't acquire (down)

Andrey


>
>>
>> Andrey
>>
>>> See Linux kernel semaphores don't allow negative values (the count
>>> field in struct semaphore is unsigned).
>> Ok, forget what I've wrote. That indeed seems to be supported, some
>> other drivers are already using semaphores the same way.
>>
>> Regards,
>> Christian.
>>
>>> Regards,
>>> Christian.
>>>
>>>> Alex
>>>>
>>>>> ---
>>>>>    include/drm/task_barrier.h | 106
>>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 106 insertions(+)
>>>>>    create mode 100644 include/drm/task_barrier.h
>>>>>
>>>>> diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h
>>>>> new file mode 100644
>>>>> index 0000000..81fb0f7
>>>>> --- /dev/null
>>>>> +++ b/include/drm/task_barrier.h
>>>>> @@ -0,0 +1,106 @@
>>>>> +/*
>>>>> + * Copyright 2019 Advanced Micro Devices, Inc.
>>>>> + *
>>>>> + * Permission is hereby granted, free of charge, to any person
>>>>> obtaining a
>>>>> + * copy of this software and associated documentation files (the
>>>>> "Software"),
>>>>> + * to deal in the Software without restriction, including without
>>>>> limitation
>>>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>>>> sublicense,
>>>>> + * and/or sell copies of the Software, and to permit persons to
>>>>> whom the
>>>>> + * Software is furnished to do so, subject to the following
>>>>> conditions:
>>>>> + *
>>>>> + * The above copyright notice and this permission notice shall be
>>>>> included in
>>>>> + * all copies or substantial portions of the Software.
>>>>> + *
>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>>> EXPRESS OR
>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
>>>>> EVENT SHALL
>>>>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>>>>> DAMAGES OR
>>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>> OTHERWISE,
>>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>>>> USE OR
>>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>>>> + *
>>>>> + */
>>>>> +#include <linux/semaphore.h>
>>>>> +#include <linux/atomic.h>
>>>>> +
>>>>> +/*
>>>>> + * Reusable 2 PHASE task barrier (randevouz point) implementation
>>>>> for N tasks.
>>>>> + * Based on the Little book of sempahores -
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgreenteapress.com%2Fwp%2Fsemaphores%2F&data=02%7C01%7CAndrey.Grodzovsky%40amd.com%7Cdcd0f1a4cfa440d7b1ae08d77f154935%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637117601707306821&sdata=X3%2Bg5XpFNrNbVka6WhB8TxjhOG1yuc%2Bk6%2FdcuO2Nlw0%3D&reserved=0 
>>>>>
>>>>> + */
>>>>> +
>>>>> +
>>>>> +
>>>>> +#ifndef DRM_TASK_BARRIER_H_
>>>>> +#define DRM_TASK_BARRIER_H_
>>>>> +
>>>>> +/*
>>>>> + * Represents an instance of a task barrier.
>>>>> + */
>>>>> +struct task_barrier {
>>>>> +       unsigned int n;
>>>>> +       atomic_t count;
>>>>> +       struct semaphore enter_turnstile;
>>>>> +       struct semaphore exit_turnstile;
>>>>> +};
>>>>> +
>>>>> +static inline void task_barrier_signal_turnstile(struct semaphore
>>>>> *turnstile,
>>>>> +                                                unsigned int n)
>>>>> +{
>>>>> +       int i;
>>>>> +
>>>>> +       for (i = 0 ; i < n; i++)
>>>>> +               up(turnstile);
>>>>> +}
>>>>> +
>>>>> +static inline void task_barrier_init(struct task_barrier *tb)
>>>>> +{
>>>>> +       tb->n = 0;
>>>>> +       atomic_set(&tb->count, 0);
>>>>> +       sema_init(&tb->enter_turnstile, 0);
>>>>> +       sema_init(&tb->exit_turnstile, 0);
>>>>> +}
>>>>> +
>>>>> +static inline void task_barrier_add_task(struct task_barrier *tb)
>>>>> +{
>>>>> +       tb->n++;
>>>>> +}
>>>>> +
>>>>> +static inline void task_barrier_rem_task(struct task_barrier *tb)
>>>>> +{
>>>>> +       tb->n--;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Lines up all the threads BEFORE the critical point.
>>>>> + *
>>>>> + * When all thread passed this code the entry barrier is back to
>>>>> locked state.
>>>>> + */
>>>>> +static inline void task_barrier_enter(struct task_barrier *tb)
>>>>> +{
>>>>> +       if (atomic_inc_return(&tb->count) == tb->n)
>>>>> + task_barrier_signal_turnstile(&tb->enter_turnstile, tb->n);
>>>>> +
>>>>> +       down(&tb->enter_turnstile);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Lines up all the threads AFTER the critical point.
>>>>> + *
>>>>> + * This function is used to avoid any one thread running ahead of
>>>>> the reset if
>>>>> + * the barrier is used in a loop (repeatedly) .
>>>>> + */
>>>>> +static inline void task_barrier_exit(struct task_barrier *tb)
>>>>> +{
>>>>> +       if (atomic_dec_return(&tb->count) == 0)
>>>>> + task_barrier_signal_turnstile(&tb->exit_turnstile, tb->n);
>>>>> +
>>>>> +       down(&tb->exit_turnstile);
>>>>> +}
>>>>> +
>>>>> +static inline void task_barrier_full(struct task_barrier *tb)
>>>>> +{
>>>>> +       task_barrier_enter(tb);
>>>>> +       task_barrier_exit(tb);
>>>>> +}
>>>>> +
>>>>> +#endif
>>>>> -- 
>>>>> 2.7.4
>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CAndrey.Grodzovsky%40amd.com%7Cdcd0f1a4cfa440d7b1ae08d77f154935%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637117601707306821&sdata=bYsDcdM0X4Mw2Q%2FQGEb9v0kXpOjqamuDVAV8jJ5YRjs%3D&reserved=0 
>>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CAndrey.Grodzovsky%40amd.com%7Cdcd0f1a4cfa440d7b1ae08d77f154935%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637117601707306821&sdata=bYsDcdM0X4Mw2Q%2FQGEb9v0kXpOjqamuDVAV8jJ5YRjs%3D&reserved=0 
>>>>
>

More information about the amd-gfx mailing list