[vlc-commits] [Git][videolan/vlc][master] 5 commits: config: make config_dirty atomic
Rémi Denis-Courmont (@Courmisch)
gitlab at videolan.org
Sat Nov 27 08:35:26 UTC 2021
Rémi Denis-Courmont pushed to branch master at VideoLAN / VLC
Commits:
b282e134 by Rémi Denis-Courmont at 2021-11-27T08:21:48+00:00
config: make config_dirty atomic
- - - - -
fd2a9dfd by Rémi Denis-Courmont at 2021-11-27T08:21:48+00:00
config: remove recursive lock on auto-save
This was only there to handle the dirty flag atomically with the actual
saving of the configuration file.
- - - - -
04e27dea by Rémi Denis-Courmont at 2021-11-27T08:21:48+00:00
config/help: fix printed default for booleans
- - - - -
dbc43dfb by Rémi Denis-Courmont at 2021-11-27T08:21:48+00:00
config: add atomic values for int and float items
The non-atomic copies are left for backward compatibility.
- - - - -
d78c6262 by Rémi Denis-Courmont at 2021-11-27T08:21:48+00:00
config: read ints and floats as atomic
This makes config_GetInt() and config_GetFloat() completely lock-less.
- - - - -
6 changed files:
- src/config/configuration.h
- src/config/core.c
- src/config/file.c
- src/config/help.c
- src/modules/cache.c
- src/modules/entry.c
Changes:
=====================================
src/config/configuration.h
=====================================
@@ -26,6 +26,11 @@
struct vlc_plugin_t;
struct vlc_param {
+ union {
+ _Atomic int64_t i; /**< Current value (if integer or boolean) */
+ _Atomic float f; /**< Current value (if floating point) */
+ } value;
+
struct vlc_plugin_t *owner;
unsigned char shortname; /**< Optional short option name */
unsigned internal:1; /**< Hidden from preferences and help */
@@ -56,7 +61,7 @@ int config_SortConfig (void);
void config_UnsortConfig (void);
extern vlc_rwlock_t config_lock;
-extern bool config_dirty;
+extern _Atomic bool config_dirty;
bool config_IsSafe (const char *);
=====================================
src/config/core.c
=====================================
@@ -24,6 +24,7 @@
# include "config.h"
#endif
+#include <stdatomic.h>
#include <vlc_common.h>
#include <vlc_actions.h>
#include <vlc_modules.h>
@@ -38,7 +39,7 @@
#include "modules/modules.h"
vlc_rwlock_t config_lock = VLC_STATIC_RWLOCK;
-bool config_dirty = false;
+atomic_bool config_dirty = ATOMIC_VAR_INIT(false);
static inline char *strdupnull (const char *src)
{
@@ -84,38 +85,26 @@ static module_config_t * config_FindConfigChecked( const char *psz_name )
return p_config;
}
-int64_t config_GetInt(const char *psz_name)
+int64_t config_GetInt(const char *name)
{
- module_config_t *p_config = config_FindConfigChecked( psz_name );
+ const struct vlc_param *param = vlc_param_Find(name);
/* sanity checks */
- assert(p_config != NULL);
- assert(IsConfigIntegerType(p_config->i_type));
+ assert(param != NULL);
+ assert(IsConfigIntegerType(param->item.i_type));
- int64_t val;
-
- vlc_rwlock_rdlock (&config_lock);
- val = p_config->value.i;
- vlc_rwlock_unlock (&config_lock);
- return val;
+ return atomic_load_explicit(¶m->value.i, memory_order_relaxed);
}
-float config_GetFloat(const char *psz_name)
+float config_GetFloat(const char *name)
{
- module_config_t *p_config;
-
- p_config = config_FindConfigChecked( psz_name );
+ const struct vlc_param *param = vlc_param_Find(name);
/* sanity checks */
- assert(p_config != NULL);
- assert(IsConfigFloatType(p_config->i_type));
+ assert(param != NULL);
+ assert(IsConfigFloatType(param->item.i_type));
- float val;
-
- vlc_rwlock_rdlock (&config_lock);
- val = p_config->value.f;
- vlc_rwlock_unlock (&config_lock);
- return val;
+ return atomic_load_explicit(¶m->value.f, memory_order_relaxed);
}
char *config_GetPsz(const char *psz_name)
@@ -151,38 +140,41 @@ void config_PutPsz(const char *psz_name, const char *psz_value)
vlc_rwlock_wrlock (&config_lock);
oldstr = (char *)p_config->value.psz;
p_config->value.psz = str;
- config_dirty = true;
vlc_rwlock_unlock (&config_lock);
+ atomic_store_explicit(&config_dirty, true, memory_order_release);
free (oldstr);
}
-void config_PutInt(const char *psz_name, int64_t i_value )
+void config_PutInt(const char *name, int64_t i_value)
{
- module_config_t *p_config = config_FindConfigChecked( psz_name );
+ struct vlc_param *param = vlc_param_Find(name);
+ module_config_t *p_config = ¶m->item;
/* sanity checks */
- assert(p_config != NULL);
- assert(IsConfigIntegerType(p_config->i_type));
+ assert(param != NULL);
+ assert(IsConfigIntegerType(param->item.i_type));
if (i_value < p_config->min.i)
i_value = p_config->min.i;
if (i_value > p_config->max.i)
i_value = p_config->max.i;
+ atomic_store_explicit(¶m->value.i, i_value, memory_order_relaxed);
vlc_rwlock_wrlock (&config_lock);
p_config->value.i = i_value;
- config_dirty = true;
vlc_rwlock_unlock (&config_lock);
+ atomic_store_explicit(&config_dirty, true, memory_order_release);
}
-void config_PutFloat(const char *psz_name, float f_value)
+void config_PutFloat(const char *name, float f_value)
{
- module_config_t *p_config = config_FindConfigChecked( psz_name );
+ struct vlc_param *param = vlc_param_Find(name);
+ module_config_t *p_config = ¶m->item;
/* sanity checks */
- assert(p_config != NULL);
- assert(IsConfigFloatType(p_config->i_type));
+ assert(param != NULL);
+ assert(IsConfigFloatType(param->item.i_type));
/* if f_min == f_max == 0, then do not use them */
if ((p_config->min.f == 0.f) && (p_config->max.f == 0.f))
@@ -192,10 +184,11 @@ void config_PutFloat(const char *psz_name, float f_value)
else if (f_value > p_config->max.f)
f_value = p_config->max.f;
+ atomic_store_explicit(¶m->value.f, f_value, memory_order_relaxed);
vlc_rwlock_wrlock (&config_lock);
p_config->value.f = f_value;
- config_dirty = true;
vlc_rwlock_unlock (&config_lock);
+ atomic_store_explicit(&config_dirty, true, memory_order_release);
}
ssize_t config_GetIntChoices(const char *name,
@@ -507,10 +500,18 @@ void config_ResetAll(void)
module_config_t *p_config = ¶m->item;
if (IsConfigIntegerType (p_config->i_type))
+ {
+ atomic_store_explicit(¶m->value.i, p_config->orig.i,
+ memory_order_relaxed);
p_config->value.i = p_config->orig.i;
+ }
else
if (IsConfigFloatType (p_config->i_type))
+ {
+ atomic_store_explicit(¶m->value.f, p_config->orig.f,
+ memory_order_relaxed);
p_config->value.f = p_config->orig.f;
+ }
else
if (IsConfigStringType (p_config->i_type))
{
@@ -520,6 +521,6 @@ void config_ResetAll(void)
}
}
}
- config_dirty = true;
vlc_rwlock_unlock (&config_lock);
+ atomic_store_explicit(&config_dirty, true, memory_order_release);
}
=====================================
src/config/file.c
=====================================
@@ -27,6 +27,7 @@
#include <errno.h> /* errno */
#include <assert.h>
#include <limits.h>
+#include <stdatomic.h>
#include <fcntl.h>
#include <sys/stat.h>
#ifdef __APPLE__
@@ -230,15 +231,25 @@ int config_LoadConfigFile( vlc_object_t *p_this )
psz_option_value, psz_option_name,
vlc_strerror_c(errno));
else
+ {
+ atomic_store_explicit(¶m->value.i, l,
+ memory_order_relaxed);
item->value.i = l;
+ }
break;
}
case CONFIG_ITEM_FLOAT:
+ {
if (!*psz_option_value)
break; /* ignore empty option */
- item->value.f = (float)atof (psz_option_value);
+
+ float f = (float)atof(psz_option_value);
+ atomic_store_explicit(¶m->value.f, f,
+ memory_order_relaxed);
+ item->value.f = f;
break;
+ }
default:
free (item->value.psz);
@@ -452,7 +463,8 @@ int config_SaveConfigFile (vlc_object_t *p_this)
if (IsConfigIntegerType (p_item->i_type))
{
- int64_t val = p_item->value.i;
+ int64_t val = atomic_load_explicit(¶m->value.i,
+ memory_order_relaxed);
config_Write (file, p_item->psz_text,
(CONFIG_CLASS(p_item->i_type) == CONFIG_ITEM_BOOL)
? N_("boolean") : N_("integer"),
@@ -462,7 +474,8 @@ int config_SaveConfigFile (vlc_object_t *p_this)
else
if (IsConfigFloatType (p_item->i_type))
{
- float val = p_item->value.f;
+ float val = atomic_load_explicit(¶m->value.f,
+ memory_order_relaxed);
config_Write (file, p_item->psz_text, N_("float"),
val == p_item->orig.f,
p_item->psz_name, "%f", val);
@@ -530,18 +543,20 @@ error:
int config_AutoSaveConfigFile( vlc_object_t *p_this )
{
- int ret = 0;
-
assert( p_this );
- vlc_rwlock_rdlock (&config_lock);
- if (config_dirty)
- {
- /* Note: this will get the read lock recursively. Ok. */
- ret = config_SaveConfigFile (p_this);
- config_dirty = (ret != 0);
- }
- vlc_rwlock_unlock (&config_lock);
+ if (!atomic_exchange_explicit(&config_dirty, false, memory_order_acquire))
+ return 0;
+
+ int ret = config_SaveConfigFile (p_this);
+
+ if (unlikely(ret != 0))
+ /*
+ * On write failure, set the dirty flag back again. While it looks
+ * racy, it really means to retry later in hope that it does not
+ * fail again. Concurrent write attempts would not succeed anyway.
+ */
+ atomic_store_explicit(&config_dirty, true, memory_order_relaxed);
return ret;
}
=====================================
src/config/help.c
=====================================
@@ -22,6 +22,7 @@
# include "config.h"
#endif
+#include <stdatomic.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
@@ -485,8 +486,8 @@ static void print_item(const module_t *m, const struct vlc_param *param,
case CONFIG_ITEM_BOOL:
bra = type = ket = "";
prefix = ", --no-";
- suffix = item->value.i ? _("(default enabled)")
- : _("(default disabled)");
+ suffix = item->orig.i ? _("(default enabled)")
+ : _("(default disabled)");
break;
default:
return;
=====================================
src/modules/cache.c
=====================================
@@ -28,6 +28,7 @@
#endif
#include <stdalign.h>
+#include <stdatomic.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
@@ -216,6 +217,12 @@ static int vlc_cache_load_config(struct vlc_param *param, block_t *file)
LOAD_IMMEDIATE (cfg->orig);
LOAD_IMMEDIATE (cfg->min);
LOAD_IMMEDIATE (cfg->max);
+ if (IsConfigFloatType(cfg->i_type))
+ atomic_store_explicit(¶m->value.f, cfg->orig.f,
+ memory_order_relaxed);
+ else
+ atomic_store_explicit(¶m->value.i, cfg->orig.i,
+ memory_order_relaxed);
cfg->value = cfg->orig;
if (cfg->list_count)
=====================================
src/modules/entry.c
=====================================
@@ -23,6 +23,7 @@
# include "config.h"
#endif
+#include <stdatomic.h>
#include <vlc_common.h>
#include <vlc_plugin.h>
#include <assert.h>
@@ -329,14 +330,18 @@ static int vlc_plugin_desc_cb(void *ctx, void *tgt, int propid, ...)
if (IsConfigIntegerType(item->i_type)
|| !CONFIG_ITEM(item->i_type))
{
- item->orig.i =
- item->value.i = va_arg (ap, int64_t);
+ item->orig.i = va_arg(ap, int64_t);
+ item->value.i = item->orig.i;
+ atomic_store_explicit(¶m->value.i, item->orig.i,
+ memory_order_relaxed);
}
else
if (IsConfigFloatType (item->i_type))
{
- item->orig.f =
- item->value.f = va_arg (ap, double);
+ item->orig.f = va_arg(ap, double);
+ item->value.f = item->orig.f;
+ atomic_store_explicit(¶m->value.f, item->orig.f,
+ memory_order_relaxed);
}
else
if (IsConfigStringType (item->i_type))
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a38f2a139ee64df6b54fba83489f660eee1f9d38...d78c6262c05b435056817657baecfcab2d9d71fc
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a38f2a139ee64df6b54fba83489f660eee1f9d38...d78c6262c05b435056817657baecfcab2d9d71fc
You're receiving this email because of your account on code.videolan.org.
More information about the vlc-commits
mailing list