Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Dec 2006 12:14:52 +0100 (CET)
From:      Martin Blapp <mb@imp.ch>
To:        Attilio Rao <attilio@FreeBSD.org>
Cc:        cvs-src@FreeBSD.org, Martin Blapp <mbr@FreeBSD.org>, cvs-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern tty.c
Message-ID:  <20061220120419.M53548@godot.imp.ch>
In-Reply-To: <3bbf2fe10612191850y3a546fcew486575385b1400df@mail.gmail.com>
References:  <200612192234.kBJMYYo0055529@repoman.freebsd.org> <3bbf2fe10612191850y3a546fcew486575385b1400df@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

Hi,

> Since proctree_lock is a sx lock which uses 2 condition variables,
> they alredy drop Giant (DROP_GIANT()) before sleeping. Are you secure
> it is the right thing to do here?

Yes I am.

sx_slock() can call cv_wait() which later can call cv_wait_unlock ...
if there is already a lock we need to share ... After a possible
sleep cv_wait_unlock looks like:

cv_wait_unlock(struct cv *cvp, struct mtx *mp)

[...]
         sleepq_lock(cvp);

         cvp->cv_waiters++;
         DROP_GIANT();
         mtx_unlock(mp);

[...]

It can call DROP_GIANT for a short amount of time. This was the race.

I already fixed it (but haven't seen the real cause for the crashes) once in 
STABLE and HEAD, but backed it out again because I (and jhb) were sure that the 
tty driver was protected fully by the GIANT lock after I fixed severall 
possible race places in kern_proc.c. I had no idea that sx_slock()  and sx_slock 
both can drop GIANT for a short time and that we need carefully recheck 
variables.

Martin



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