Attachment 662504 Details for Bug 883429 – Macro to deal with memcpy barriers

[patch] Macro to deal with memcpy barriers

0001-Add-a-macro-to-copy-with-barriers.patch (text/plain), 4.00 KB, created by Simo Sorce on 2012-12-12 17:11:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simo Sorce

Created: 2012-12-12 17:11:33 UTC

Size: 4.00 KB

patch

obsolete

>From c884ad3be20c084dd214756829370c594b3c1b88 Mon Sep 17 00:00:00 2001
>From: Simo Sorce <simo@redhat.com>
>Date: Tue, 11 Dec 2012 17:23:19 -0500
>Subject: [PATCH] Add a macro to copy with barriers
>
>We have 2 places where we memcpy memory and need barriers protection.
>Use a macro so we can consolidate code in one place.
>
>Second fix for: https://fedorahosted.org/sssd/ticket/1694
>---
> src/sss_client/nss_mc_common.c | 47 +++++++++++++++++++++++++++---------------
> 1 file changed, 30 insertions(+), 17 deletions(-)
>
>diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
>index a361f57b9794cad0cded6c3bbad5f66543edb31b..59f5b257b9c646aa826499623103cd2c45abdbc7 100644
>--- a/src/sss_client/nss_mc_common.c
>+++ b/src/sss_client/nss_mc_common.c
>@@ -38,18 +38,33 @@
> /* FIXME: handle name upper/lower casing ? Maybe a flag passed down by
>  * sssd or a flag in sss_mc_header ? per domain ? */
> 
>+#define MEMCPY_WITH_BARRIERS(res, dest, src, len) \
>+do { \
>+    uint32_t _b1; \
>+    res = false; \
>+    _b1 = (src)->b1; \
>+    if (MC_VALID_BARRIER(_b1)) { \
>+        __sync_synchronize(); \
>+        memcpy(dest, src, len); \
>+        __sync_synchronize(); \
>+        if ((src)->b2 == _b1) { \
>+            res = true; \
>+        } \
>+    } \
>+} while(0)
>+
> errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
> {
>     struct sss_mc_header h;
>+    bool copy_ok;
>     int count;
> 
>     /* retry barrier protected reading max 5 times then give up */
>     for (count = 5; count > 0; count--) {
>-        memcpy(&h, ctx->mmap_base, sizeof(struct sss_mc_header));
>-        /* we need a barrier here to make sure the compiler does not optimize
>-         * too much and avoids updating the register for the next check */
>-        __sync_synchronize();
>-        if (MC_VALID_BARRIER(h.b1) && h.b1 == h.b2) {
>+        MEMCPY_WITH_BARRIERS(copy_ok, &h,
>+                             (struct sss_mc_header *)ctx->mmap_base,
>+                             sizeof(struct sss_mc_header));
>+        if (copy_ok) {
>             /* record is consistent so we can proceed */
>             break;
>         }
>@@ -181,11 +196,12 @@ errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx,
>                               uint32_t slot, struct sss_mc_rec **_rec)
> {
>     struct sss_mc_rec *rec;
>-    void *rec_buf = NULL;
>+    struct sss_mc_rec *copy_rec = NULL;
>     size_t buf_size = 0;
>     size_t rec_len;
>     uint32_t b1;
>     uint32_t b2;
>+    bool copy_ok;
>     int count;
>     int ret;
> 
>@@ -198,16 +214,16 @@ errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx,
>         __sync_synchronize();
>         rec_len = rec->len;
>         __sync_synchronize();
>-        b2 = rec->b1;
>+        b2 = rec->b2;
>         if (!MC_VALID_BARRIER(b1) || b1 != b2) {
>             /* record is inconsistent, retry */
>             continue;
>         }
> 
>         if (rec_len > buf_size) {
>-            free(rec_buf);
>-            rec_buf = malloc(rec_len);
>-            if (!rec_buf) {
>+            free(copy_rec);
>+            copy_rec = malloc(rec_len);
>+            if (!copy_rec) {
>                 ret = ENOMEM;
>                 goto done;
>             }
>@@ -215,13 +231,10 @@ errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx,
>         }
>         /* we cannot access data directly, we must copy data and then
>          * access the copy */
>-        memcpy(rec_buf, rec, rec_len);
>-        rec = (struct sss_mc_rec *)rec_buf;
>+        MEMCPY_WITH_BARRIERS(copy_ok, copy_rec, rec, rec_len);
> 
>         /* we must check data is consistent again after the copy */
>-        if (MC_VALID_BARRIER(rec->b1) &&
>-            rec->b1 == rec->b2 &&
>-            rec->len == rec_len) {
>+        if (copy_ok && b1 == copy_rec->b2) {
>             /* record is consistent, use it */
>             break;
>         }
>@@ -232,12 +245,12 @@ errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx,
>         goto done;
>     }
> 
>-    *_rec = rec;
>+    *_rec = copy_rec;
>     ret = 0;
> 
> done:
>     if (ret) {
>-        free(rec_buf);
>+        free(copy_rec);
>         *_rec = NULL;
>     }
>     return ret;
>-- 
>1.7.11.7
>

Actions: View | Diff

Attachments on bug 883429: 660930 | 661730 | 662504