Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 24 Oct 2008 18:01:14 +0200
From:      "Attilio Rao" <attilio@freebsd.org>
To:        "David Xu" <davidxu@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r184216 - head/sys/kern
Message-ID:  <3bbf2fe10810240901s576f6dday9b59ad44f0a1a85@mail.gmail.com>
In-Reply-To: <200810240103.m9O13V7f071075@svn.freebsd.org>
References:  <200810240103.m9O13V7f071075@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
2008/10/24 David Xu <davidxu@freebsd.org>:
> Author: davidxu
> Date: Fri Oct 24 01:03:31 2008
> New Revision: 184216
> URL: http://svn.freebsd.org/changeset/base/184216
>
> Log:
>  partly revert revision 184199, because TDF_NEEDSIGCHK is persitent
>  when thread is in kernel mode, it can cause dead loop, now unlock
>  process lock after acquired sleep queue lock and thread lock to
>  avoid the problem. This means TDF_NEEDSIGCHK and TDF_NEEDSUSPCHK must
>  be set with process lock and thread lock being hold at same time.
>
> Modified:
>  head/sys/kern/subr_sleepqueue.c
>
> Modified: head/sys/kern/subr_sleepqueue.c
> ==============================================================================
> --- head/sys/kern/subr_sleepqueue.c     Thu Oct 23 21:50:16 2008        (r184215)
> +++ head/sys/kern/subr_sleepqueue.c     Fri Oct 24 01:03:31 2008        (r184216)
> @@ -396,7 +396,6 @@ sleepq_catch_signals(void *wchan, int pr
>                return (0);
>        }
>
> -catch_sig:
>        thread_unlock(td);
>        mtx_unlock_spin(&sc->sc_lock);
>        CTR3(KTR_PROC, "sleepq catching signals: thread %p (pid %ld, %s)",
> @@ -416,19 +415,15 @@ catch_sig:
>                        ret = ERESTART;
>                mtx_unlock(&ps->ps_mtx);
>        }
> -       PROC_UNLOCK(p);
>
>        mtx_lock_spin(&sc->sc_lock);
>        thread_lock(td);
> -       if (ret != 0)
> -               goto out;
> -       if ((td->td_flags & (TDF_NEEDSIGCHK | TDF_NEEDSUSPCHK)) != 0)
> -               goto catch_sig;
> -
> -       sleepq_switch(wchan, pri);
> -       return (0);
> +       PROC_UNLOCK(p);
> +       if (ret == 0) {
> +               sleepq_switch(wchan, pri);
> +               return (0);
> +       }
>
> -out:
>        /*
>         * There were pending signals and this thread is still
>         * on the sleep queue, remove it from the sleep queue.
>

As long as this is a variation about the usual scheme in the td_flags
locking, you should document that with a comment IMHO.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10810240901s576f6dday9b59ad44f0a1a85>