Hi Honnappa,
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Honnappa Nagarahalli
> Sent: Tuesday, April 23, 2019 12:31
> To: konstantin.anan...@intel.com; step...@networkplumber.org;
> paul...@linux.ibm.com; marko.kovace...@intel.com; dev@dpdk.org
> Cc: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; Gavin Hu (Arm
> Technology China) <gavin...@arm.com>; Dharmik Thakkar
> <dharmik.thak...@arm.com>; Malvika Gupta <malvika.gu...@arm.com>
> Subject: [dpdk-dev] [PATCH v7 1/3] rcu: add RCU library supporting QSBR
> mechanism
>
*snip*
> +int __rte_experimental
> +rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr *v, unsigned int
> +thread_id) {
> + unsigned int i, id, success;
> + uint64_t old_bmap, new_bmap;
> +
> + if (v == NULL || thread_id >= v->max_threads) {
> + rte_log(RTE_LOG_ERR, rcu_log_type,
> + "%s(): Invalid input parameter\n", __func__);
> + rte_errno = EINVAL;
> +
> + return 1;
> + }
> +
> + RCU_IS_LOCK_CNT_ZERO(v, thread_id, ERR, "Lock counter %u\n",
> + v->qsbr_cnt[thread_id].lock_cnt);
> +
> + id = thread_id & RTE_QSBR_THRID_MASK;
> + i = thread_id >> RTE_QSBR_THRID_INDEX_SHIFT;
> +
> + /* Make sure that the counter for registered threads does not
> + * go out of sync. Hence, additional checks are required.
> + */
> + /* Check if the thread is already unregistered */
> + old_bmap = __atomic_load_n(RTE_QSBR_THRID_ARRAY_ELM(v, i),
> + __ATOMIC_RELAXED);
> + if (old_bmap & ~(1UL << id))
If I understand correctly, here should be (!(old_bmap & 1UL << id))
Can you please check?
> + return 0;
> +
> + do {
> + new_bmap = old_bmap & ~(1UL << id);
> + /* Make sure any loads of the shared data structure are
> + * completed before removal of the thread from the list of
> + * reporting threads.
> + */
> + success = __atomic_compare_exchange(
> + RTE_QSBR_THRID_ARRAY_ELM(v, i),
> + &old_bmap, &new_bmap, 0,
> + __ATOMIC_RELEASE,
> __ATOMIC_RELAXED);
> +
> + if (success)
> + __atomic_fetch_sub(&v->num_threads,
> + 1, __ATOMIC_RELAXED);
> + else if (old_bmap & ~(1UL << id))
Same comment as previous.
> + /* Someone else unregistered this thread.
> + * Counter should not be incremented.
> + */
> + return 0;
> + } while (success == 0);
> +
> + return 0;
> +}
*snip*
