Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 13 Sep 2012 01:07:11 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Konstantin Belousov <kostikbel@gmail.com>, Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   Re: svn commit: r238907 - projects/calloutng/sys/kern
Message-ID:  <CAJ-FndDnYE3CKNRajGn5YV4TBKDRCnX3=cgyT2yA9mxbnkangQ@mail.gmail.com>
In-Reply-To: <201209121511.42296.jhb@freebsd.org>
References:  <201207301350.q6UDobCI099069@svn.freebsd.org> <504CF1FB.9090106@FreeBSD.org> <CAJ-FndCW91Y=SB3GFXDxtXGQdQzUzyF5KzKqjtuFYGs0W0-w6g@mail.gmail.com> <201209121511.42296.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 12, 2012 at 8:11 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Monday, September 10, 2012 9:11:22 pm Attilio Rao wrote:
>> Speaking of which, I think it is time for curthread != NULL checks in
>> the locking primitives to go, or there is a good reason I really don't
>> understand to keep them?
>
> They can probably be axed.

What do you think about this?
Please note that I would also axe the check in printtrap() on several
arches, but maybe there is a valid reason to keep it I'm not thinking
right now, so I left it out in this patch.

Thanks,
Attilio


Subject: [PATCH 6/6] Remove all the checks on curthread != NULL with the
 exception of MD in printtrap() functions on several
 !x86 arches.

Generally this check is not needed anymore, as there is not
a legitimate case where curthread != NULL.

Discussed with: bde, jhb
---
 sys/dev/hwpmc/hwpmc_arm.c |    6 ++----
 sys/dev/hwpmc/hwpmc_x86.c |    3 +--
 sys/kern/kern_condvar.c   |    2 +-
 sys/kern/kern_mutex.c     |    5 -----
 sys/kern/kern_rwlock.c    |    2 --
 sys/kern/kern_sx.c        |    5 -----
 sys/kern/kern_thread.c    |    1 -
 sys/kern/vfs_subr.c       |    1 -
 8 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/sys/dev/hwpmc/hwpmc_arm.c b/sys/dev/hwpmc/hwpmc_arm.c
index c2c24c2..79613f7 100644
--- a/sys/dev/hwpmc/hwpmc_arm.c
+++ b/sys/dev/hwpmc/hwpmc_arm.c
@@ -78,8 +78,7 @@ pmc_save_kernel_callchain(uintptr_t *cc, int maxsamples,
        pc = PMC_TRAPFRAME_TO_PC(tf);
        *cc++ = pc;

-       if ((td = curthread) == NULL)
-               return (1);
+       td = curthread;

        if (maxsamples <= 1)
                return (1);
@@ -129,8 +128,7 @@ pmc_save_user_callchain(uintptr_t *cc, int maxsamples,
        pc = PMC_TRAPFRAME_TO_PC(tf);
        *cc++ = pc;

-       if ((td = curthread) == NULL)
-               return (1);
+       td = curthread;

        if (maxsamples <= 1)
                return (1);
diff --git a/sys/dev/hwpmc/hwpmc_x86.c b/sys/dev/hwpmc/hwpmc_x86.c
index e7485a4..cd9e4f6 100644
--- a/sys/dev/hwpmc/hwpmc_x86.c
+++ b/sys/dev/hwpmc/hwpmc_x86.c
@@ -168,8 +168,7 @@ pmc_save_kernel_callchain(uintptr_t *cc, int
nframes, struct trapframe *tf)
        *cc++ = pc;
        r = fp + sizeof(uintptr_t); /* points to return address */

-       if ((td = curthread) == NULL)
-               return (1);
+       td = curthread;

        if (nframes <= 1)
                return (1);
diff --git a/sys/kern/kern_condvar.c b/sys/kern/kern_condvar.c
index 9355819..630075b 100644
--- a/sys/kern/kern_condvar.c
+++ b/sys/kern/kern_condvar.c
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
  * Common sanity checks for cv_wait* functions.
  */
 #define        CV_ASSERT(cvp, lock, td) do {
         \
-       KASSERT((td) != NULL, ("%s: curthread NULL", __func__));        \
+       KASSERT((td) != NULL, ("%s: passed td is NULL", __func__));     \
        KASSERT(TD_IS_RUNNING(td), ("%s: not TDS_RUNNING", __func__));  \
        KASSERT((cvp) != NULL, ("%s: cvp NULL", __func__));             \
        KASSERT((lock) != NULL, ("%s: lock NULL", __func__));           \
diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c
index 9827a9f..2ab7a09 100644
--- a/sys/kern/kern_mutex.c
+++ b/sys/kern/kern_mutex.c
@@ -200,7 +200,6 @@ _mtx_lock_flags(struct mtx *m, int opts, const
char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("mtx_lock() by idle thread %p on sleep mutex %s @ %s:%d",
            curthread, m->lock_object.lo_name, file, line));
@@ -225,7 +224,6 @@ _mtx_unlock_flags(struct mtx *m, int opts, const
char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(m->mtx_lock != MTX_DESTROYED,
            ("mtx_unlock() of destroyed mutex @ %s:%d", file, line));
        KASSERT(LOCK_CLASS(&m->lock_object) == &lock_class_mtx_sleep,
@@ -248,7 +246,6 @@ _mtx_lock_spin_flags(struct mtx *m, int opts,
const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(m->mtx_lock != MTX_DESTROYED,
            ("mtx_lock_spin() of destroyed mutex @ %s:%d", file, line));
        KASSERT(LOCK_CLASS(&m->lock_object) == &lock_class_mtx_spin,
@@ -272,7 +269,6 @@ _mtx_unlock_spin_flags(struct mtx *m, int opts,
const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(m->mtx_lock != MTX_DESTROYED,
            ("mtx_unlock_spin() of destroyed mutex @ %s:%d", file, line));
        KASSERT(LOCK_CLASS(&m->lock_object) == &lock_class_mtx_spin,
@@ -303,7 +299,6 @@ mtx_trylock_flags_(struct mtx *m, int opts, const
char *file, int line)
        if (SCHEDULER_STOPPED())
                return (1);

-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("mtx_trylock() by idle thread %p on sleep mutex %s @ %s:%d",
            curthread, m->lock_object.lo_name, file, line));
diff --git a/sys/kern/kern_rwlock.c b/sys/kern/kern_rwlock.c
index e0be154..3a51874 100644
--- a/sys/kern/kern_rwlock.c
+++ b/sys/kern/kern_rwlock.c
@@ -241,7 +241,6 @@ _rw_wlock(struct rwlock *rw, const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("rw_wlock() by idle thread %p on rwlock %s @ %s:%d",
            curthread, rw->lock_object.lo_name, file, line));
@@ -292,7 +291,6 @@ _rw_wunlock(struct rwlock *rw, const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(rw->rw_lock != RW_DESTROYED,
            ("rw_wunlock() of destroyed rwlock @ %s:%d", file, line));
        _rw_assert(rw, RA_WLOCKED, file, line);
diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c
index 487a324..af2391f 100644
--- a/sys/kern/kern_sx.c
+++ b/sys/kern/kern_sx.c
@@ -249,7 +249,6 @@ _sx_slock(struct sx *sx, int opts, const char
*file, int line)

        if (SCHEDULER_STOPPED())
                return (0);
-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("sx_slock() by idle thread %p on sx %s @ %s:%d",
            curthread, sx->lock_object.lo_name, file, line));
@@ -303,7 +302,6 @@ _sx_xlock(struct sx *sx, int opts, const char
*file, int line)

        if (SCHEDULER_STOPPED())
                return (0);
-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("sx_xlock() by idle thread %p on sx %s @ %s:%d",
            curthread, sx->lock_object.lo_name, file, line));
@@ -330,7 +328,6 @@ sx_try_xlock_(struct sx *sx, const char *file, int line)
        if (SCHEDULER_STOPPED())
                return (1);

-       MPASS(curthread != NULL);
        KASSERT(!TD_IS_IDLETHREAD(curthread),
            ("sx_try_xlock() by idle thread %p on sx %s @ %s:%d",
            curthread, sx->lock_object.lo_name, file, line));
@@ -361,7 +358,6 @@ _sx_sunlock(struct sx *sx, const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(sx->sx_lock != SX_LOCK_DESTROYED,
            ("sx_sunlock() of destroyed sx @ %s:%d", file, line));
        _sx_assert(sx, SA_SLOCKED, file, line);
@@ -378,7 +374,6 @@ _sx_xunlock(struct sx *sx, const char *file, int line)

        if (SCHEDULER_STOPPED())
                return;
-       MPASS(curthread != NULL);
        KASSERT(sx->sx_lock != SX_LOCK_DESTROYED,
            ("sx_xunlock() of destroyed sx @ %s:%d", file, line));
        _sx_assert(sx, SA_XLOCKED, file, line);
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index c4ad7b8..7c21644 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -622,7 +622,6 @@ thread_single(int mode)
        p = td->td_proc;
        mtx_assert(&Giant, MA_NOTOWNED);
        PROC_LOCK_ASSERT(p, MA_OWNED);
-       KASSERT((td != NULL), ("curthread is NULL"));

        if ((p->p_flag & P_HADTHREADS) == 0)
                return (0);
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 06d16c0..5c781c2 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -3416,7 +3416,6 @@ vfs_unmountall(void)
        struct thread *td;
        int error;

-       KASSERT(curthread != NULL, ("vfs_unmountall: NULL curthread"));
        CTR1(KTR_VFS, "%s: unmounting all filesystems", __func__);
        td = curthread;

--
1.7.7.4



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndDnYE3CKNRajGn5YV4TBKDRCnX3=cgyT2yA9mxbnkangQ>