[vlc-devel] [PATCH 1/7] posix: add fallback vlc_atomic_* implementation
Rémi Denis-Courmont
remi at remlab.net
Mon Feb 17 15:09:44 CET 2020
The intent is to provide an implementation of the APIs to fall back on. What it already says.
Le 17 février 2020 15:58:07 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>Hi,
>
>On Mon, Feb 17, 2020 at 03:35:40PM +0200, 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.
>>
>
>You could probably describe the intent (in which case is it need
>and why?) and give a general guideline to help review.
>
>What you add is always supposed to be written in the diff section
>of the patch itself or the naming is bad. Here it is correct but
>I feel the intent and guidelines could be helpful.
>
>Regards,
>--
>Alexandre Janniaux
>Videolabs
>
>
>> 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.0
>> >>
>> >> _______________________________________________
>> >> vlc-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
>_______________________________________________
>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/49d06035/attachment-0001.html>
More information about the vlc-devel
mailing list