Author: sbruno
Date: Fri Jun  5 18:16:10 2015
New Revision: 284043
URL: https://svnweb.freebsd.org/changeset/base/284043

Log:
  Revert 284029, update imgact_binmisctl.c change mtx to reader count, at the
  request of the submitter.
  
  Will attempt to use an sx_lock for this fix to WITNESS crashes in a later
  revision.
  
  Submitted by: sson

Modified:
  head/sys/kern/imgact_binmisc.c

Modified: head/sys/kern/imgact_binmisc.c
==============================================================================
--- head/sys/kern/imgact_binmisc.c      Fri Jun  5 17:26:07 2015        
(r284042)
+++ head/sys/kern/imgact_binmisc.c      Fri Jun  5 18:16:10 2015        
(r284043)
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2013-15, Stacey D. Son
+ * Copyright (c) 2013, Stacey D. Son
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41,9 +41,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/malloc.h>
 #include <sys/mutex.h>
 #include <sys/sysctl.h>
-#include <sys/sx.h>
-
-#include <machine/atomic.h>
 
 /**
  * Miscellaneous binary interpreter image activator.
@@ -98,68 +95,11 @@ static SLIST_HEAD(, imgact_binmisc_entry
        SLIST_HEAD_INITIALIZER(interpreter_list);
 
 static int interp_list_entry_count = 0;
-static int interp_list_rcount = 0;
+
 static struct mtx interp_list_mtx;
 
 int imgact_binmisc_exec(struct image_params *imgp);
 
-static inline void
-interp_list_rlock()
-{
-
-    /* Don't use atomic_add() here.  We must block if the mutex is held. */
-    mtx_lock(&interp_list_mtx);
-    interp_list_rcount++;
-    mtx_unlock(&interp_list_mtx);
-}
-
-static inline void
-interp_list_runlock()
-{
-
-    /* Decrement the reader count and wake up one writer, if any. */
-    atomic_subtract_int(&interp_list_rcount, 1);
-    if (interp_list_rcount == 0)
-        wakeup_one(&interp_list_rcount);
-}
-
-static inline void
-interp_list_wlock()
-{
-
-    /* Wait until there are no readers. */
-    mtx_lock(&interp_list_mtx);
-    while (interp_list_rcount)
-        mtx_sleep(&interp_list_rcount, &interp_list_mtx, 0, "IABM", 0);
-}
-
-static inline void
-interp_list_wunlock()
-{
-
-    mtx_unlock(&interp_list_mtx);
-}
-
-#define interp_list_assert_rlocked()                                     \
-       KASSERT(interp_list_rcount > 0, ("%s: interp_list lock not held " \
-                                       "for read", __func__))
-
-#define        interp_list_assert_wlocked() mtx_assert(&interp_list_mtx, 
MA_OWNED)
-
-#ifdef INVARIANTS
-#define        interp_list_assert_locked()     do {                            
\
-       if (interp_list_rcount == 0)                                    \
-               mtx_assert(&interp_list_mtx, MA_OWNED);                 \
-       else                                                            \
-               KASSERT(interp_list_rcount > 0,                         \
-                       ("%s: interp_list lock not held", __func__));   \
-} while(0)
-
-#else /* ! INVARIANTS */
-
-#define        interp_list_assert_locked()     do {                            
\
-} while(0)
-#endif /* ! INVARIANTS */
 
 /*
  * Populate the entry with the information about the interpreter.
@@ -171,7 +111,7 @@ imgact_binmisc_populate_interp(char *str
        char t[IBE_INTERP_LEN_MAX];
        char *sp, *tp;
 
-       memset(t, 0, sizeof(t));
+       bzero(t, sizeof(t));
 
        /*
         * Normalize interpreter string. Replace white space between args with
@@ -212,6 +152,8 @@ imgact_binmisc_new_entry(ximgact_binmisc
        imgact_binmisc_entry_t *ibe = NULL;
        size_t namesz = min(strlen(xbe->xbe_name) + 1, IBE_NAME_MAX);
 
+       mtx_assert(&interp_list_mtx, MA_NOTOWNED);
+
        ibe = malloc(sizeof(*ibe), M_BINMISC, M_WAITOK|M_ZERO);
 
        ibe->ibe_name = malloc(namesz, M_BINMISC, M_WAITOK|M_ZERO);
@@ -261,7 +203,7 @@ imgact_binmisc_find_entry(char *name)
 {
        imgact_binmisc_entry_t *ibe;
 
-       interp_list_assert_locked();
+       mtx_assert(&interp_list_mtx, MA_OWNED);
 
        SLIST_FOREACH(ibe, &interpreter_list, link) {
                if (strncmp(name, ibe->ibe_name, IBE_NAME_MAX) == 0)
@@ -318,21 +260,21 @@ imgact_binmisc_add_entry(ximgact_binmisc
                }
        }
 
-       /* Preallocate a new entry. */
-       ibe = imgact_binmisc_new_entry(xbe);
-       if (!ibe)
-               return (ENOMEM);
-
-       interp_list_wlock();
+       mtx_lock(&interp_list_mtx);
        if (imgact_binmisc_find_entry(xbe->xbe_name) != NULL) {
-               interp_list_wunlock();
-               imgact_binmisc_destroy_entry(ibe);
+               mtx_unlock(&interp_list_mtx);
                return (EEXIST);
        }
+       mtx_unlock(&interp_list_mtx);
+
+       ibe = imgact_binmisc_new_entry(xbe);
+       if (!ibe)
+               return (ENOMEM);
 
+       mtx_lock(&interp_list_mtx);
        SLIST_INSERT_HEAD(&interpreter_list, ibe, link);
        interp_list_entry_count++;
-       interp_list_wunlock();
+       mtx_unlock(&interp_list_mtx);
 
        return (0);
 }
@@ -346,14 +288,14 @@ imgact_binmisc_remove_entry(char *name)
 {
        imgact_binmisc_entry_t *ibe;
 
-       interp_list_wlock();
+       mtx_lock(&interp_list_mtx);
        if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-               interp_list_wunlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOENT);
        }
        SLIST_REMOVE(&interpreter_list, ibe, imgact_binmisc_entry, link);
        interp_list_entry_count--;
-       interp_list_wunlock();
+       mtx_unlock(&interp_list_mtx);
 
        imgact_binmisc_destroy_entry(ibe);
 
@@ -369,14 +311,14 @@ imgact_binmisc_disable_entry(char *name)
 {
        imgact_binmisc_entry_t *ibe;
 
-       interp_list_rlock();
+       mtx_lock(&interp_list_mtx);
        if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOENT);
        }
 
-       atomic_clear_32(&ibe->ibe_flags, IBF_ENABLED);
-       interp_list_runlock();
+       ibe->ibe_flags &= ~IBF_ENABLED;
+       mtx_unlock(&interp_list_mtx);
 
        return (0);
 }
@@ -390,14 +332,14 @@ imgact_binmisc_enable_entry(char *name)
 {
        imgact_binmisc_entry_t *ibe;
 
-       interp_list_rlock();
+       mtx_lock(&interp_list_mtx);
        if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOENT);
        }
 
-       atomic_set_32(&ibe->ibe_flags, IBF_ENABLED);
-       interp_list_runlock();
+       ibe->ibe_flags |= IBF_ENABLED;
+       mtx_unlock(&interp_list_mtx);
 
        return (0);
 }
@@ -408,9 +350,9 @@ imgact_binmisc_populate_xbe(ximgact_binm
 {
        uint32_t i;
 
-       interp_list_assert_rlocked();
+       mtx_assert(&interp_list_mtx, MA_OWNED);
 
-       memset(xbe, 0, sizeof(*xbe));
+       bzero(xbe, sizeof(*xbe));
        strlcpy(xbe->xbe_name, ibe->ibe_name, IBE_NAME_MAX);
 
        /* Copy interpreter string.  Replace NULL breaks with space. */
@@ -440,14 +382,14 @@ imgact_binmisc_lookup_entry(char *name, 
        imgact_binmisc_entry_t *ibe;
        int error = 0;
 
-       interp_list_rlock();
+       mtx_lock(&interp_list_mtx);
        if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOENT);
        }
 
        error = imgact_binmisc_populate_xbe(xbe, ibe);
-       interp_list_runlock();
+       mtx_unlock(&interp_list_mtx);
 
        return (error);
 }
@@ -462,12 +404,12 @@ imgact_binmisc_get_all_entries(struct sy
        imgact_binmisc_entry_t *ibe;
        int error = 0, count;
 
-       interp_list_rlock();
+       mtx_lock(&interp_list_mtx);
        count = interp_list_entry_count;
        /* Don't block in malloc() while holding lock. */
        xbe = malloc(sizeof(*xbe) * count, M_BINMISC, M_NOWAIT|M_ZERO);
        if (!xbe) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOMEM);
        }
 
@@ -477,7 +419,7 @@ imgact_binmisc_get_all_entries(struct sy
                if (error)
                        break;
        }
-       interp_list_runlock();
+       mtx_unlock(&interp_list_mtx);
 
        if (!error)
                error = SYSCTL_OUT(req, xbe, sizeof(*xbe) * count);
@@ -614,7 +556,7 @@ imgact_binmisc_find_interpreter(const ch
        int i;
        size_t sz;
 
-       interp_list_assert_rlocked();
+       mtx_assert(&interp_list_mtx, MA_OWNED);
 
        SLIST_FOREACH(ibe, &interpreter_list, link) {
                if (!(IBF_ENABLED & ibe->ibe_flags))
@@ -651,15 +593,15 @@ imgact_binmisc_exec(struct image_params 
        char *s, *d;
 
        /* Do we have an interpreter for the given image header? */
-       interp_list_rlock();
+       mtx_lock(&interp_list_mtx);
        if ((ibe = imgact_binmisc_find_interpreter(image_header)) == NULL) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (-1);
        }
 
        /* No interpreter nesting allowed. */
        if (imgp->interpreted & IMGACT_BINMISC) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                return (ENOEXEC);
        }
 
@@ -707,7 +649,7 @@ imgact_binmisc_exec(struct image_params 
 
                default:
                        /* Hmm... This shouldn't happen. */
-                       interp_list_runlock();
+                       mtx_unlock(&interp_list_mtx);
                        printf("%s: Unknown macro #%c sequence in "
                            "interpreter string\n", KMOD_NAME, *(s + 1));
                        error = EINVAL;
@@ -718,15 +660,12 @@ imgact_binmisc_exec(struct image_params 
 
        /* Check to make sure we won't overrun the stringspace. */
        if (offset > imgp->args->stringspace) {
-               interp_list_runlock();
+               mtx_unlock(&interp_list_mtx);
                error = E2BIG;
                goto done;
        }
 
-       /*
-        * Make room for the interpreter. Doing an overlapping copy so
-        * bcopy() must be used.
-        */
+       /* Make room for the interpreter */
        bcopy(imgp->args->begin_argv, imgp->args->begin_argv + offset,
            imgp->args->endp - imgp->args->begin_argv);
 
@@ -781,7 +720,7 @@ imgact_binmisc_exec(struct image_params 
                s++;
        }
        *d = '\0';
-       interp_list_runlock();
+       mtx_unlock(&interp_list_mtx);
 
        if (!error)
                imgp->interpreter_name = imgp->args->begin_argv;
@@ -806,13 +745,13 @@ imgact_binmisc_fini(void *arg)
        imgact_binmisc_entry_t *ibe, *ibe_tmp;
 
        /* Free all the interpreters. */
-       interp_list_wlock();
+       mtx_lock(&interp_list_mtx);
        SLIST_FOREACH_SAFE(ibe, &interpreter_list, link, ibe_tmp) {
                SLIST_REMOVE(&interpreter_list, ibe, imgact_binmisc_entry,
                    link);
                imgact_binmisc_destroy_entry(ibe);
        }
-       interp_list_wunlock();
+       mtx_unlock(&interp_list_mtx);
 
        mtx_destroy(&interp_list_mtx);
 }
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to