[vlc-devel] [PATCH 1/7] posix: add fallback vlc_atomic_* implementation
Rémi Denis-Courmont
remi at remlab.net
Mon Feb 17 15:00:28 CET 2020
The description already says it's a fallback implementation for POSIX. There's nothing else to this patch than that and the existing API documentation.
Le 17 février 2020 15:47:52 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>On Mon, Feb 17, 2020, at 14:35, Rémi Denis-Courmont wrote:
>> I want the APIs to work on all platforms, and this adds a generic
>POSIX implementation. Exactly what it says already. I don't know what
>else you want to explain.
>
>So why you don't write it in the commit log ?
>
>Maybe it's just me, but I had trouble understanding this set.
>Explaining why you do a commit can be very helpful. Specially on set
>like this where you get the reason why only after the 4th or 5th
>commit.
>
>Commit messages should explain 'why'. The 'what' should be in the
>diff/code/comment.
>
>>
>> Le 17 février 2020 14:39:08 GMT+02:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>>>
>>>
>>> On Sun, Feb 16, 2020, at 18:50, Rémi Denis-Courmont wrote:
>>>> src/Makefile.am | 3 ++
>>>> src/posix/wait.c | 128
>+++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 131 insertions(+)
>>>> create mode 100644 src/posix/wait.c
>>>>
>>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>>> index f2adf61095..0ad47fc105 100644
>>>> --- a/src/Makefile.am
>>>> +++ b/src/Makefile.am
>>>> @@ -482,6 +482,9 @@ libvlccore_la_SOURCES += \
>>>> posix/plugin.c \
>>>> posix/rand.c \
>>>> posix/timer.c
>>>> +if !HAVE_LINUX
>>>> +libvlccore_la_SOURCES += posix/wait.c
>>>> +endif
>>>> if !HAVE_ANDROID
>>>> libvlccore_la_SOURCES += posix/sort.c
>>>> if !HAVE_DARWIN
>>>> diff --git a/src/posix/wait.c b/src/posix/wait.c
>>>> new file mode 100644
>>>> index 0000000000..60f9c7626f
>>>> --- /dev/null
>>>> +++ b/src/posix/wait.c
>>>> @@ -0,0 +1,128 @@
>>>>
>+/*****************************************************************************
>>>> + * wait.c : pthread back-end for vlc_atomic_wait
>>>> +
>*****************************************************************************
>>>> + * Copyright (C) 2019-2020 Rémi Denis-Courmont
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>modify it
>>>> + * under the terms of the GNU Lesser General Public License as
>published by
>>>> + * the Free Software Foundation; either version 2.1 of the
>License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General
>Public License
>>>> + * along with this program; if not, write to the Free Software
>Foundation,
>>>> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301,
>USA.
>>>> +
>*****************************************************************************/
>>>> +
>>>> +#ifdef HAVE_CONFIG_H
>>>> +# include "config.h"
>>>> +#endif
>>>> +
>>>> +#include <vlc_common.h>
>>>> +
>>>> +#include <stdalign.h>
>>>> +#include <stdatomic.h>
>>>> +#include <errno.h>
>>>> +#include <time.h>
>>>> +
>>>> +#include <sys/types.h>
>>>> +#include <pthread.h>
>>>> +
>>>> +#define WAIT_BUCKET_INIT \
>>>> + { PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, 0 }
>>>> +#define WAIT_BUCKET_INIT_2 WAIT_BUCKET_INIT, WAIT_BUCKET_INIT
>>>> +#define WAIT_BUCKET_INIT_4 WAIT_BUCKET_INIT_2, WAIT_BUCKET_INIT_2
>>>> +#define WAIT_BUCKET_INIT_8 WAIT_BUCKET_INIT_4, WAIT_BUCKET_INIT_4
>>>> +#define WAIT_BUCKET_INIT_16 WAIT_BUCKET_INIT_8, WAIT_BUCKET_INIT_8
>>>> +#define WAIT_BUCKET_INIT_32 WAIT_BUCKET_INIT_16,
>WAIT_BUCKET_INIT_16
>>>
>>>
>>> If I understand correctly, this bucket is here to minimize the
>number of spurious wake, but it doesn't prevent it from happening,
>which is fine ?
>>>
>>>> +
>>>> +static struct wait_bucket
>>>> +{
>>>> + pthread_mutex_t lock;
>>>> + pthread_cond_t wait;
>>>> + unsigned waiters;
>>>> +} wait_buckets[32] = { WAIT_BUCKET_INIT_32 };
>>>> +
>>>> +static struct wait_bucket *wait_bucket_get(atomic_uint *addr)
>>>> +{
>>>> + uintptr_t u = (uintptr_t)addr;
>>>> + size_t idx = (u / alignof (*addr)) % ARRAY_SIZE(wait_buckets);
>>>> +
>>>> + return &wait_buckets[idx];
>>>> +}
>>>> +
>>>> +static struct wait_bucket *wait_bucket_enter(atomic_uint *addr)
>>>> +{
>>>> + struct wait_bucket *bucket = wait_bucket_get(addr);
>>>> +
>>>> + pthread_mutex_lock(&bucket->lock);
>>>> + bucket->waiters++;
>>>> + return bucket;
>>>> +}
>>>> +
>>>> +static void wait_bucket_leave(void *data)
>>>> +{
>>>> + struct wait_bucket *bucket = data;
>>>> +
>>>> + bucket->waiters--;
>>>> + pthread_mutex_unlock(&bucket->lock);
>>>> +}
>>>> +
>>>> +void vlc_atomic_wait(void *addr, unsigned value)
>>>> +{
>>>> + atomic_uint *futex = addr;
>>>> + struct wait_bucket *bucket = wait_bucket_enter(futex);
>>>> +
>>>> + pthread_cleanup_push(wait_bucket_leave, bucket);
>>>> +
>>>> + if (value == atomic_load_explicit(futex,
>memory_order_relaxed))
>>>> + pthread_cond_wait(&bucket->wait, &bucket->lock);
>>>> +
>>>> + pthread_cleanup_pop(1);
>>>> +}
>>>> +
>>>> +bool vlc_atomic_timedwait(void *addr, unsigned value, vlc_tick_t
>delay)
>>>> +{
>>>> + atomic_uint *futex = addr;
>>>> + struct wait_bucket *bucket;
>>>> + struct timespec ts;
>>>> + lldiv_t d = lldiv(delay, CLOCK_FREQ);
>>>> + int ret = 0;
>>>> +
>>>> + clock_gettime(CLOCK_MONOTONIC, &ts);
>>>> + ts.tv_sec += d.quot;
>>>> + ts.tv_nsec += NS_FROM_VLC_TICK(d.rem);
>>>> +
>>>> + if (ts.tv_nsec >= 1000000000) {
>>>> + ts.tv_sec++;
>>>> + ts.tv_nsec -= 1000000000;
>>>> + }
>>>> +
>>>> + bucket = wait_bucket_enter(futex);
>>>> + pthread_cleanup_push(wait_bucket_leave, bucket);
>>>> +
>>>> + if (value == atomic_load_explicit(futex,
>memory_order_relaxed))
>>>> + ret = pthread_cond_timedwait(&bucket->wait, &bucket->lock,
>&ts);
>>>> +
>>>> + pthread_cleanup_pop(1);
>>>> + return ret == 0;
>>>> +}
>>>> +
>>>> +void vlc_atomic_notify_one(void *addr)
>>>> +{
>>>> + vlc_atomic_notify_all(addr);
>>>> +}
>>>> +
>>>> +void vlc_atomic_notify_all(void *addr)
>>>> +{
>>>> + struct wait_bucket *bucket = wait_bucket_get(addr);
>>>> +
>>>> + pthread_mutex_lock(&bucket->lock);
>>>> + if (bucket->waiters > 0)
>>>> + pthread_cond_broadcast(&bucket->wait);
>>>> + pthread_mutex_unlock(&bucket->lock);
>>>> +}
>>>
>>> OK for me, but it deserves a git message and/or few comments. It
>took me some time to understand what you wanted to do.
>>>
>>>> --
>>>> 2.25.0vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>> --
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté.
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200217/44602de7/attachment.html>
More information about the vlc-devel
mailing list