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 mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to