Re: [Freeipa-devel] [PATCH 0384-0385] Replace isc_atomic_* in with reference counter
On 23.6.2015 14:18, Tomas Hozza wrote: On 23.06.2015 11:32, Petr Spacek wrote: On 10.6.2015 19:07, Petr Spacek wrote: Hello, Replace isc_atomic_* in MetaLDAP with reference counter abstraction. + Replace isc_atomic_* in instance tainting with reference counter abstraction. Reference counters are used as abstraction which hides missing isc_atomic_*() functions on some architectures. This change is necessary for architectures like s390x and ppc64le where BIND does not provide isc_atomic_* abstractions. Fixed version of the patch is attached. The same code is also on Github: https://github.com/pspacek/bind-dyndb-ldap/commits/atomic_to_refcnt Thank you for review! I did formal review of patches 384 and 385. The fixed version looks good. ACK. Thanks, pushed to master: 1f167ee3366d7cc65038141640670dd0771c333f 0946ef7d9e15ad46b603ef10fb352d9743d06ee6 -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0384-0385] Replace isc_atomic_* in with reference counter
On 23.06.2015 11:32, Petr Spacek wrote: On 10.6.2015 19:07, Petr Spacek wrote: Hello, Replace isc_atomic_* in MetaLDAP with reference counter abstraction. + Replace isc_atomic_* in instance tainting with reference counter abstraction. Reference counters are used as abstraction which hides missing isc_atomic_*() functions on some architectures. This change is necessary for architectures like s390x and ppc64le where BIND does not provide isc_atomic_* abstractions. Fixed version of the patch is attached. The same code is also on Github: https://github.com/pspacek/bind-dyndb-ldap/commits/atomic_to_refcnt Thank you for review! I did formal review of patches 384 and 385. The fixed version looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0384-0385] Replace isc_atomic_* in with reference counter
On 10.6.2015 19:07, Petr Spacek wrote:
Hello,
Replace isc_atomic_* in MetaLDAP with reference counter abstraction.
+
Replace isc_atomic_* in instance tainting with reference counter abstraction.
Reference counters are used as abstraction which hides missing isc_atomic_*()
functions on some architectures.
This change is necessary for architectures like s390x and ppc64le where BIND
does not provide isc_atomic_* abstractions.
Fixed version of the patch is attached.
The same code is also on Github:
https://github.com/pspacek/bind-dyndb-ldap/commits/atomic_to_refcnt
Thank you for review!
--
Petr^2 Spacek
From 1f167ee3366d7cc65038141640670dd0771c333f Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 10 Jun 2015 16:51:14 +0200
Subject: [PATCH] Replace isc_atomic_* in instance tainting with reference
counter abstraction.
Reference counters are used as abstraction which hides missing isc_atomic_*()
functions on some architectures.
---
src/ldap_helper.c | 49 ++---
src/ldap_helper.h | 6 ++
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d6461a3e83b63555a46ff3f60761e3703d9a6b4e..415786d31776d8780f44e75f48674c47a2f61b21 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -24,7 +24,6 @@
#include dns/soa.h
#include dns/update.h
-#include isc/atomic.h
#include isc/buffer.h
#include isc/dir.h
#include isc/mem.h
@@ -37,6 +36,7 @@
#include isc/util.h
#include isc/netaddr.h
#include isc/parseint.h
+#include isc/refcount.h
#include isc/timer.h
#include isc/serial.h
#include isc/string.h
@@ -159,10 +159,8 @@ struct ldap_instance {
isc_task_t *task;
isc_thread_t watcher;
isc_boolean_t exiting;
- /* Non-zero if this instance 'tainted' by a unrecoverable problem.
- * It should be accessed using isc_atomic_*() because it might be
- * modified from multiple threads. */
- isc_int32_t tainted;
+ /* Non-zero if this instance is 'tainted' by an unrecoverable problem. */
+ isc_refcount_t errors;
/* Settings. */
settings_set_t *local_settings;
@@ -517,6 +515,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
CHECKED_MEM_GET_PTR(mctx, ldap_inst);
ZERO_PTR(ldap_inst);
+ CHECK(isc_refcount_init(ldap_inst-errors, 0));
isc_mem_attach(mctx, ldap_inst-mctx);
ldap_inst-db_name = db_name;
@@ -663,6 +662,10 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
settings_set_free(ldap_inst-local_settings);
sync_ctx_free(ldap_inst-sctx);
+ /* zero out error counter (and do nothing other than that) */
+ ldap_instance_untaint_finish(ldap_inst,
+ ldap_instance_untaint_start(ldap_inst));
+ isc_refcount_destroy(ldap_inst-errors);
MEM_PUT_AND_DETACH(ldap_inst);
@@ -4684,10 +4687,42 @@ ldap_instance_isexiting(ldap_instance_t *ldap_inst)
* (if it is even possible). */
void
ldap_instance_taint(ldap_instance_t *ldap_inst) {
- isc_atomic_store(ldap_inst-tainted, 1);
+ isc_refcount_increment0(ldap_inst-errors, NULL);
}
isc_boolean_t
ldap_instance_istained(ldap_instance_t *ldap_inst) {
- return ISC_TF(isc_atomic_cmpxchg(ldap_inst-tainted, 0, 0) != 0);
+ return ISC_TF(isc_refcount_current(ldap_inst-errors) != 0);
+}
+
+/**
+ * Get number of errors from LDAP instance. This function should be called
+ * before re-synchronization with LDAP is started.
+ * When the re-synchronization is finished, the result of this function
+ * has to be passed to ldap_instance_untaint_finish() to detect if any other
+ * error occurred during the re-synchronization.
+ */
+unsigned int
+ldap_instance_untaint_start(ldap_instance_t *ldap_inst) {
+ return isc_refcount_current(ldap_inst-errors);
+}
+
+/**
+ * @retval DNS_R_CONTINUE An error occurred during re-synchronization,
+ *it is necessary to start again.
+ * @retval ISC_R_SUCCESS Number of errors at the beginning and the end of
+ *re-sychronization matches so no new errors occurred
+ *during re-synchronization.
+ */
+isc_result_t
+ldap_instance_untaint_finish(ldap_instance_t *ldap_inst, unsigned int count) {
+ unsigned int remaining = 0;
+ while (count 0) {
+ isc_refcount_decrement(ldap_inst-errors, remaining);
+ count--;
+ }
+ if (remaining != 0)
+ return DNS_R_CONTINUE;
+ else
+ return ISC_R_SUCCESS;
}
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index e81b8aa59d3518b80afec2ad357e859bcb7eac20..b4b1ee59edb3414b305888271dc425980a1fd3df 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -90,4 +90,10 @@ isc_boolean_t ldap_instance_isexiting(ldap_instance_t *ldap_inst) ATTR_NONNULLS
void ldap_instance_taint(ldap_instance_t *ldap_inst) ATTR_NONNULLS;
+unsigned int
+ldap_instance_untaint_start(ldap_instance_t *ldap_inst);
+
+isc_result_t
+ldap_instance_untaint_finish(ldap_instance_t *ldap_inst, unsigned int count);
+
#endif /* !_LD_LDAP_HELPER_H_ */
--
2.1.0
--
Manage your subscription for the
[Freeipa-devel] [PATCH 0384-0385] Replace isc_atomic_* in with reference counter
Hello,
Replace isc_atomic_* in MetaLDAP with reference counter abstraction.
+
Replace isc_atomic_* in instance tainting with reference counter abstraction.
Reference counters are used as abstraction which hides missing isc_atomic_*()
functions on some architectures.
This change is necessary for architectures like s390x and ppc64le where BIND
does not provide isc_atomic_* abstractions.
--
Petr^2 Spacek
From 1221199b195c39143ce9d193163241739e93354f Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 10 Jun 2015 16:51:14 +0200
Subject: [PATCH] Replace isc_atomic_* in instance tainting with reference
counter abstraction.
Reference counters are used as abstraction which hides missing isc_atomic_*()
functions on some architectures.
---
src/ldap_helper.c | 40
src/ldap_helper.h | 6 ++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d6461a3e83b63555a46ff3f60761e3703d9a6b4e..6804acf95b74528277093f26236f57f4aa0b7d05 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -24,7 +24,6 @@
#include dns/soa.h
#include dns/update.h
-#include isc/atomic.h
#include isc/buffer.h
#include isc/dir.h
#include isc/mem.h
@@ -37,6 +36,7 @@
#include isc/util.h
#include isc/netaddr.h
#include isc/parseint.h
+#include isc/refcount.h
#include isc/timer.h
#include isc/serial.h
#include isc/string.h
@@ -162,7 +162,7 @@ struct ldap_instance {
/* Non-zero if this instance 'tainted' by a unrecoverable problem.
* It should be accessed using isc_atomic_*() because it might be
* modified from multiple threads. */
- isc_int32_t tainted;
+ isc_refcount_t errors;
/* Settings. */
settings_set_t *local_settings;
@@ -517,6 +517,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
CHECKED_MEM_GET_PTR(mctx, ldap_inst);
ZERO_PTR(ldap_inst);
+ CHECK(isc_refcount_init(ldap_inst-errors, 0));
isc_mem_attach(mctx, ldap_inst-mctx);
ldap_inst-db_name = db_name;
@@ -663,6 +664,10 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
settings_set_free(ldap_inst-local_settings);
sync_ctx_free(ldap_inst-sctx);
+ /* zero out error counter (and do nothing other than that) */
+ ldap_instance_untaint_finish(ldap_inst,
+ ldap_instance_untaint_start(ldap_inst));
+ isc_refcount_destroy(ldap_inst-errors);
MEM_PUT_AND_DETACH(ldap_inst);
@@ -4684,10 +4689,37 @@ ldap_instance_isexiting(ldap_instance_t *ldap_inst)
* (if it is even possible). */
void
ldap_instance_taint(ldap_instance_t *ldap_inst) {
- isc_atomic_store(ldap_inst-tainted, 1);
+ isc_refcount_increment0(ldap_inst-errors, NULL);
}
isc_boolean_t
ldap_instance_istained(ldap_instance_t *ldap_inst) {
- return ISC_TF(isc_atomic_cmpxchg(ldap_inst-tainted, 0, 0) != 0);
+ return ISC_TF(isc_refcount_current(ldap_inst-errors) != 0);
+}
+
+/**
+ * Pass result of this function to ldap_instance_untaint_finish().
+ */
+unsigned int
+ldap_instance_untaint_start(ldap_instance_t *ldap_inst) {
+ unsigned int errors;
+ errors = isc_refcount_current(ldap_inst-errors);
+
+ return ISC_TF(errors != 0);
+}
+
+/**
+ * DNS_R_CONTINUE: untainting was not finished - start again.
+ */
+isc_result_t
+ldap_instance_untaint_finish(ldap_instance_t *ldap_inst, unsigned int count) {
+ unsigned int remaining = 0;
+ while (count 0) {
+ isc_refcount_decrement(ldap_inst-errors, remaining);
+ count--;
+ }
+ if (remaining != 0)
+ return DNS_R_CONTINUE;
+ else
+ return ISC_R_SUCCESS;
}
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index e81b8aa59d3518b80afec2ad357e859bcb7eac20..b4b1ee59edb3414b305888271dc425980a1fd3df 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -90,4 +90,10 @@ isc_boolean_t ldap_instance_isexiting(ldap_instance_t *ldap_inst) ATTR_NONNULLS
void ldap_instance_taint(ldap_instance_t *ldap_inst) ATTR_NONNULLS;
+unsigned int
+ldap_instance_untaint_start(ldap_instance_t *ldap_inst);
+
+isc_result_t
+ldap_instance_untaint_finish(ldap_instance_t *ldap_inst, unsigned int count);
+
#endif /* !_LD_LDAP_HELPER_H_ */
--
2.1.0
From f91cfee843dab9adac2626d88f11566993f58562 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 10 Jun 2015 18:25:19 +0200
Subject: [PATCH] Replace isc_atomic_* in MetaLDAP with reference counter
abstraction.
Reference counters are used as abstraction which hides missing isc_atomic_*()
functions on some architectures.
---
src/mldap.c | 47 +--
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/src/mldap.c b/src/mldap.c
index 0c8327ccd7be802c9ee97838d19efb57715328fc..8cffe8a1fbf8eaa20aae79c28ad8d7a305494f19 100644
--- a/src/mldap.c
+++ b/src/mldap.c
@@ -11,6 +11,7 @@
#include isc/boolean.h
#include isc/net.h
+#include isc/refcount.h
#include isc/result.h
#include isc/util.h
#include isc/serial.h
@@ -47,7 +48,7 @@ static dns_name_t uuid_rootname =
struct mldapdb {
isc_mem_t *mctx;
metadb_t
