On Tue, Jan 26, 2016 at 05:35:06PM +0100, Pavel Reichl wrote:
Hello,
please see simple patch attached.
Hi Pavel,
sorry for the delay. See comments below.
bye,
Sumit
Thanks.
From 8ea41341650498aade9ce91ac04501327d89a558 Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 26 Jan 2016 11:28:22 -0500
Subject: [PATCH] IDMAP: Add minor performance improvements
Some ID ranges are precalculated when ID mapping is being initialized.
This patch utilizes these (helper) ranges when new domains are generated
if appropriate.
---
src/lib/idmap/sss_idmap.c | 96 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 85 insertions(+), 11 deletions(-)
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 269ef0132ff3b9ffbfbe65006361fac6d4f88cf9..394f9dcee69f7340e3af907fabd612b9ab205df8
100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -89,6 +89,48 @@ static char *idmap_strdup(struct sss_idmap_ctx *ctx, const char *str)
return new;
}
+static bool ranges_eq(const struct idmap_range_params *a,
+ const struct idmap_range_params *b)
+{
+ if (a == NULL || b == NULL) {
+ return false;
+ }
+
+ if (a->first_rid == b->first_rid
+ && a->min_id == b->min_id
+ && a->max_id == b->max_id) {
+ return true;
+ }
+
+ return false;
+}
+
+static struct idmap_range_params*
+construct_range(struct sss_idmap_ctx *ctx,
+ const struct idmap_range_params *src,
+ char *id)
+{
+ struct idmap_range_params *dst;
+
+ if (src == NULL || id == NULL) {
+ return NULL;
+ }
+
+ dst = ctx->alloc_func(sizeof(struct idmap_range_params), ctx->alloc_pvt);
+ if (dst == NULL) {
+ return NULL;
+ }
+
+ /* Copy */
+ dst->min_id = src->min_id;
+ dst->max_id = src->max_id;
+ dst->first_rid = src->first_rid;
+ dst->next = src->next;
I think next is better set to NULL as it is done in generate_slice(), or
is there a specific reason here?
+
+ dst->range_id = id;
+ return dst;
+}
+
static bool id_is_in_range(uint32_t id,
struct idmap_range_params *rp,
uint32_t *rid)
@@ -232,6 +274,18 @@ static void free_helpers(struct sss_idmap_ctx *ctx,
}
}
+static struct idmap_range_params*
+get_helper_by_id(struct idmap_range_params *helpers, const char *id)
+{
+ for (struct idmap_range_params *it = helpers; it != NULL; it = it->next) {
imo moving the declaration of 'it' out of the for-loop header would make
it more readable.
+ if (strcmp(it->range_id, id) == 0) {
+ return it;
+ }
+ }
+
+ return NULL;
+}
+
static void sss_idmap_free_domain(struct sss_idmap_ctx *ctx,
struct idmap_domain_info *dom)
{
@@ -846,30 +900,48 @@ static bool comp_id(struct idmap_range_params *range_params, long
long rid,
static enum idmap_error_code
get_range(struct sss_idmap_ctx *ctx,
+ struct idmap_range_params *helpers,
const char *dom_sid,
long long rid,
struct idmap_range_params **_range)
{
- char *secondary_name;
+ char *secondary_name = NULL;;
enum idmap_error_code err;
int first_rid;
struct idmap_range_params *range;
+ struct idmap_range_params *helper;
first_rid = (rid / ctx->idmap_opts.rangesize) * ctx->idmap_opts.rangesize;
secondary_name = generate_sec_slice_name(ctx, dom_sid, first_rid);
if (secondary_name == NULL) {
- return IDMAP_OUT_OF_MEMORY;
+ err = IDMAP_OUT_OF_MEMORY;
+ goto error;
}
- err = generate_slice(ctx, secondary_name, first_rid, &range);
- if (err == IDMAP_OUT_OF_SLICES) {
- ctx->free_func(secondary_name, ctx->alloc_pvt);
- return err;
+ helper = get_helper_by_id(helpers, secondary_name);
+ if (helper != NULL) {
+ /* Utilize helper's range. */
+ range = construct_range(ctx, helper, secondary_name);
If construct_range() would return idmap_error_code as generate_slice()
you can handle the errors in a common if-block.
+ if (range == NULL) {
+ err = IDMAP_OUT_OF_MEMORY;
+ goto error;
+ }
+ range->next = NULL;
+ } else {
+ /* Have to generate a whole new range. */
+ err = generate_slice(ctx, secondary_name, first_rid, &range);
+ if (err == IDMAP_OUT_OF_SLICES) {
I realized that generate_slice() can return IDMAP_OUT_OF_MEMORY as well,
so checking for err != IDMAP_SUCCESS might be better and would allow to
handle construct_range() as well (see above).
+ goto error;
+ }
}
*_range = range;
return IDMAP_SUCCESS;
+
+error:
+ ctx->free_func(secondary_name, ctx->alloc_pvt);
+ return err;
}
static enum idmap_error_code
@@ -896,9 +968,7 @@ spawn_dom(struct sss_idmap_ctx *ctx,
it = ctx->idmap_domain_info;
while (it != NULL) {
/* Find the newly added domain. */
- if (it->range_params.first_rid == range->first_rid
- && it->range_params.min_id == range->min_id
- && it->range_params.max_id == range->max_id) {
+ if (ranges_eq(&it->range_params, range)) {
/* Share helpers. */
it->helpers = parent->helpers;
@@ -950,8 +1020,7 @@ add_dom_for_sid(struct sss_idmap_ctx *ctx,
goto done;
}
- /* todo optimize */
- err = get_range(ctx, matched_dom->sid, rid, &range);
+ err = get_range(ctx, matched_dom->helpers, matched_dom->sid, rid,
&range);
if (err != IDMAP_SUCCESS) {
goto done;
}
@@ -1135,6 +1204,11 @@ enum idmap_error_code sss_idmap_unix_to_sid(struct sss_idmap_ctx
*ctx,
it != NULL;
it = it->next) {
+ if (idmap_domain_info->helpers_owner == false) {
+ /* Checking helpers on owner is sufficient. */
+ continue;
+ }
+
if (id_is_in_range(id, it, &rid)) {
if (idmap_domain_info->external_mapping == true
--
2.4.3
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org