[ltt-dev] [PATCH 3/9] rculfhash: make struct rcu_level allied
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Mon Oct 17 09:35:50 EDT 2011
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> On 10/14/2011 10:07 PM, Mathieu Desnoyers wrote:
> > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >> reduce memory fragment
> >> add a slowpath overhead
> >>
> >> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >
> > Hrm, I see what you want to do here: given we already have
> > synchronize_rcu calls in the fini code, you use them to guarantee
> > quiescence of the previous iteration instead of using call_rcu. I would
> > argue that at this point in the development process, I would rather
> > prefer to keep allocation/delayed free as decoupled as possible from the
> > algorithm. So I'll keep this optimisation in mind, but for now I think I
> > will keep the call_rcu scheme to deal with all memory free. This makes
> > it more easy to modify the code when the allocation/delayed free
> > concerns are all encapsulated through use of call_rcu.
> >
> > Moreover, this patch really is an optimisation, so it should be backed
> > by benchmarks that shows if there are measurable performance
> > improvements.
>
> (1UL << order) sizes of memory is good for memory allocator,
> especial when we port this into kernel.
> slowpath overhead is not overhead for me.
>
> more important, it makes small tables mergeable.
> See next round patches.
>
> I assume this patch will be merged until you reject the related
> patches in the next round.
Merged as:
commit 0d14ceb26e0ff3e1d6f86cf3ec9674ee3b16e96f
Author: Lai Jiangshan <laijs at cn.fujitsu.com>
Date: Mon Oct 17 09:32:00 2011 -0400
rculfhash: make struct rcu_level size power of 2
By adding a small slowpath overhead (added synchronize_rcu call in the
last iteration of the resize), we can reduce the amount of wasted memory
for memory allocators that allocate power of two memory areas. This is
achieved by removing the call_rcu head from struct rcu_level.
[ Edit by Mathieu Desnoyers:
- add comment about need to manually update the allocation size of
fields are added to struct rcu_level.
- create a more explanatory title and changelog. ]
Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
diff --git a/rculfhash.c b/rculfhash.c
index fe8beed..2e338f1 100644
--- a/rculfhash.c
+++ b/rculfhash.c
@@ -209,7 +209,7 @@ struct ht_items_count {
} __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
struct rcu_level {
- struct rcu_head head;
+ /* Note: manually update allocation length when adding a field */
struct _cds_lfht_node nodes[0];
};
@@ -716,14 +716,6 @@ unsigned long _uatomic_max(unsigned long *ptr, unsigned long v)
return v;
}
-static
-void cds_lfht_free_level(struct rcu_head *head)
-{
- struct rcu_level *l =
- caa_container_of(head, struct rcu_level, head);
- poison_free(l);
-}
-
/*
* Remove all logically deleted nodes from a bucket up to a certain node key.
*/
@@ -1135,8 +1127,7 @@ void init_table(struct cds_lfht *ht,
if (CMM_LOAD_SHARED(ht->t.resize_target) < (!i ? 1 : (1UL << i)))
break;
- ht->t.tbl[i] = calloc(1, sizeof(struct rcu_level)
- + (len * sizeof(struct _cds_lfht_node)));
+ ht->t.tbl[i] = calloc(1, len * sizeof(struct _cds_lfht_node));
assert(ht->t.tbl[i]);
/*
@@ -1222,6 +1213,7 @@ void fini_table(struct cds_lfht *ht,
unsigned long first_order, unsigned long len_order)
{
long i, end_order;
+ void *free_by_rcu = NULL;
dbg_printf("fini table: first_order %lu end_order %lu\n",
first_order, first_order + len_order);
@@ -1247,6 +1239,8 @@ void fini_table(struct cds_lfht *ht,
* return a logically removed node as insert position.
*/
ht->cds_lfht_synchronize_rcu();
+ if (free_by_rcu)
+ free(free_by_rcu);
/*
* Set "removed" flag in dummy nodes about to be removed.
@@ -1256,12 +1250,17 @@ void fini_table(struct cds_lfht *ht,
*/
remove_table(ht, i, len);
- ht->cds_lfht_call_rcu(&ht->t.tbl[i]->head, cds_lfht_free_level);
+ free_by_rcu = ht->t.tbl[i];
dbg_printf("fini new size: %lu\n", 1UL << i);
if (CMM_LOAD_SHARED(ht->in_progress_destroy))
break;
}
+
+ if (free_by_rcu) {
+ ht->cds_lfht_synchronize_rcu();
+ free(free_by_rcu);
+ }
}
struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
>
> Thanks.
> Lai
>
> >
> > Thanks!
> >
> > Mathieu
> >
> >> ---
> >> rculfhash.c | 22 ++++++++++------------
> >> 1 files changed, 10 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/rculfhash.c b/rculfhash.c
> >> index e379c71..49f3637 100644
> >> --- a/rculfhash.c
> >> +++ b/rculfhash.c
> >> @@ -209,7 +209,6 @@ struct ht_items_count {
> >> } __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
> >>
> >> struct rcu_level {
> >> - struct rcu_head head;
> >> struct _cds_lfht_node nodes[0];
> >> };
> >>
> >> @@ -712,14 +711,6 @@ unsigned long _uatomic_max(unsigned long *ptr, unsigned long v)
> >> return v;
> >> }
> >>
> >> -static
> >> -void cds_lfht_free_level(struct rcu_head *head)
> >> -{
> >> - struct rcu_level *l =
> >> - caa_container_of(head, struct rcu_level, head);
> >> - poison_free(l);
> >> -}
> >> -
> >> /*
> >> * Remove all logically deleted nodes from a bucket up to a certain node key.
> >> */
> >> @@ -1131,8 +1122,7 @@ void init_table(struct cds_lfht *ht,
> >> if (CMM_LOAD_SHARED(ht->t.resize_target) < (!i ? 1 : (1UL << i)))
> >> break;
> >>
> >> - ht->t.tbl[i] = calloc(1, sizeof(struct rcu_level)
> >> - + (len * sizeof(struct _cds_lfht_node)));
> >> + ht->t.tbl[i] = calloc(1, len * sizeof(struct _cds_lfht_node));
> >> assert(ht->t.tbl[i]);
> >>
> >> /*
> >> @@ -1218,6 +1208,7 @@ void fini_table(struct cds_lfht *ht,
> >> unsigned long first_order, unsigned long len_order)
> >> {
> >> long i, end_order;
> >> + void *free_by_rcu = NULL;
> >>
> >> dbg_printf("fini table: first_order %lu end_order %lu\n",
> >> first_order, first_order + len_order);
> >> @@ -1243,6 +1234,8 @@ void fini_table(struct cds_lfht *ht,
> >> * return a logically removed node as insert position.
> >> */
> >> ht->cds_lfht_synchronize_rcu();
> >> + if (free_by_rcu)
> >> + free(free_by_rcu);
> >>
> >> /*
> >> * Set "removed" flag in dummy nodes about to be removed.
> >> @@ -1252,12 +1245,17 @@ void fini_table(struct cds_lfht *ht,
> >> */
> >> remove_table(ht, i, len);
> >>
> >> - ht->cds_lfht_call_rcu(&ht->t.tbl[i]->head, cds_lfht_free_level);
> >> + free_by_rcu = ht->t.tbl[i];
> >>
> >> dbg_printf("fini new size: %lu\n", 1UL << i);
> >> if (CMM_LOAD_SHARED(ht->in_progress_destroy))
> >> break;
> >> }
> >> +
> >> + if (free_by_rcu) {
> >> + ht->cds_lfht_synchronize_rcu();
> >> + free(free_by_rcu);
> >> + }
> >> }
> >>
> >> struct cds_lfht *_cds_lfht_new(cds_lfht_hash_fct hash_fct,
> >> --
> >> 1.7.4.4
> >>
> >
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list