[lttng-dev] [PATCH 00/12] rculfhash memory managements

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> 3fd3f554f6eb18ae5ec526b82025a53a554f775d are wrong,
> 
> 1) dynamic len tbl_chunk must at the end of cds_lfht, (so I put all the mm field at the end of cds_lfht).

My bad, will fix.

> 
> 2) lists of hot access fields:
> size
> tbl_order/chunk/mmap
> bucket_at
> flags (testing for counting)
> split_count
> min_alloc_buckets_order (bucket_at() for chunk mm)
> min_nr_alloc_buckets (buket_at() for order mm and chunk mm)
> 
> They are many, so I group them by functional, not accessing ratio. 

Here is the fix. Please note that we could probably do better by
putting the fast-path fields into a cacheline-aligned structure. I'm
just unsure about the effect of putting the union with a variable-sized
field into such a structure, so I'll let you look into this if you want.

Here is my fix:

commit f45b03e03cc09b539b6f6c4f6a72663c212343d8
Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Date:   Mon Nov 28 09:37:21 2011 -0500

    Fix cds_lfht field order
    
    * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
    > 3fd3f554f6eb18ae5ec526b82025a53a554f775d are wrong,
    >
    > 1) dynamic len tbl_chunk must at the end of cds_lfht, (so I put all the mm field at the end of cds_lfht).
    
    My bad, will fix.
    
    >
    > 2) lists of hot access fields:
    > size
    > tbl_order/chunk/mmap
    > bucket_at
    > flags (testing for counting)
    > split_count
    > min_alloc_buckets_order (bucket_at() for chunk mm)
    > min_nr_alloc_buckets (buket_at() for order mm and chunk mm)
    
    Suggested-by: Lai Jiangshan <laijs at cn.fujitsu.com>
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

diff --git a/rculfhash-internal.h b/rculfhash-internal.h
index 792e04d..38a0317 100644
--- a/rculfhash-internal.h
+++ b/rculfhash-internal.h
@@ -54,12 +54,54 @@ struct ht_items_count;
  * cds_lfht: Top-level data structure representing a lock-free hash
  * table. Defined in the implementation file to make it be an opaque
  * cookie to users.
+ *
+ * The fields used in fast-paths are placed near the end of the
+ * structure, because we need to have a variable-sized union to contain
+ * the mm plugin fields, which are used in the fast path.
  */
 struct cds_lfht {
+	/* Initial configuration items */
+	unsigned long max_nr_buckets;
+	const struct cds_lfht_mm_type *mm;	/* memory management plugin */
+	const struct rcu_flavor_struct *flavor;	/* RCU flavor */
+
+	long count;			/* global approximate item count */
+
+	/*
+	 * We need to put the work threads offline (QSBR) when taking this
+	 * mutex, because we use synchronize_rcu within this mutex critical
+	 * section, which waits on read-side critical sections, and could
+	 * therefore cause grace-period deadlock if we hold off RCU G.P.
+	 * completion.
+	 */
+	pthread_mutex_t resize_mutex;	/* resize mutex: add/del mutex */
+	pthread_attr_t *resize_attr;	/* Resize threads attributes */
+	unsigned int in_progress_resize, in_progress_destroy;
+	unsigned long resize_target;
+	int resize_initiated;
+
+	/*
+	 * Variables needed for add and remove fast-paths.
+	 */
+	int flags;
+	unsigned long min_alloc_buckets_order;
+	unsigned long min_nr_alloc_buckets;
+	struct ht_items_count *split_count;	/* split item count */
+
 	/*
 	 * Variables needed for the lookup, add and remove fast-paths.
 	 */
 	unsigned long size;	/* always a power of 2, shared (RCU) */
+	/*
+	 * bucket_at pointer is kept here to skip the extra level of
+	 * dereference needed to get to "mm" (this is a fast-path).
+	 */
+	struct cds_lfht_node *(*bucket_at)(struct cds_lfht *ht,
+			unsigned long index);
+	/*
+	 * Dynamic length "tbl_chunk" needs to be at the end of
+	 * cds_lfht.
+	 */
 	union {
 		/*
 		 * Contains the per order-index-level bucket node table.
@@ -90,42 +132,9 @@ struct cds_lfht {
 		struct cds_lfht_node *tbl_mmap;
 	};
 	/*
-	 * bucket_at pointer is kept here to skip the extra level of
-	 * dereference needed to get to "mm" (this is a fast-path).
-	 */
-	struct cds_lfht_node *(*bucket_at)(struct cds_lfht *ht,
-			unsigned long index);
-	/*
-	 * End of variables needed for lookup fast-path.
-	 */
-
-	struct ht_items_count *split_count;	/* split item count */
-	/*
-	 * End of variables needed for add/remove fast-path.
-	 */
-
-	/* Initial configuration items */
-	int flags;
-	unsigned long min_alloc_buckets_order;
-	unsigned long min_nr_alloc_buckets;
-	unsigned long max_nr_buckets;
-	const struct cds_lfht_mm_type *mm;	/* memory management plugin */
-	const struct rcu_flavor_struct *flavor;	/* RCU flavor */
-
-	long count;			/* global approximate item count */
-
-	/*
-	 * We need to put the work threads offline (QSBR) when taking this
-	 * mutex, because we use synchronize_rcu within this mutex critical
-	 * section, which waits on read-side critical sections, and could
-	 * therefore cause grace-period deadlock if we hold off RCU G.P.
-	 * completion.
+	 * End of variables needed for the lookup, add and remove
+	 * fast-paths.
 	 */
-	pthread_mutex_t resize_mutex;	/* resize mutex: add/del mutex */
-	pthread_attr_t *resize_attr;	/* Resize threads attributes */
-	unsigned int in_progress_resize, in_progress_destroy;
-	unsigned long resize_target;
-	int resize_initiated;
 };
 
 extern unsigned int fls_ulong(unsigned long x);


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com