On 01/26/2016 05:35 PM, Pavel Reichl wrote:
Hello,
please see simple patch attached.
Thanks.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
Bump.
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@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@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On 02/15/2016 06:19 PM, Sumit Bose wrote:
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 for checking and for comments. Amended patch is attached.
Bye.
On Wed, Feb 17, 2016 at 10:47:26AM +0100, Pavel Reichl wrote:
On 02/15/2016 06:19 PM, Sumit Bose wrote:
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 for checking and for comments. Amended patch is attached.
Bye.
Thank you, I have no further comments. I didn't found any issues or regressions in my testing and CI http://sssd-ci.duckdns.org/logs/job/38/33/summary.html passed as well, so ACK.
bye, Sumit
On Mon, Feb 29, 2016 at 03:10:05PM +0100, Sumit Bose wrote:
On Wed, Feb 17, 2016 at 10:47:26AM +0100, Pavel Reichl wrote:
On 02/15/2016 06:19 PM, Sumit Bose wrote:
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 for checking and for comments. Amended patch is attached.
Bye.
Thank you, I have no further comments. I didn't found any issues or regressions in my testing and CI http://sssd-ci.duckdns.org/logs/job/38/33/summary.html passed as well, so ACK.
bye, Sumit
* master: 012d334cec221d8abf86dffbbaf9649ec0a4b585
sssd-devel@lists.fedorahosted.org