Converting to condvar(9) [Was: Re: CVS commit: src/sys/dev/ieee1394]

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Christoph Egger <Christoph_Egger%gmx.de@localhost> wrote:
> >> Log Message:
> >> don't call tsleep() with any mutexes hold
> >>
> >>
> >> To generate a diff of this commit:
> >> cvs rdiff -u -r1.23 -r1.24 src/sys/dev/ieee1394/fwdev.c
> > 
> > The right fix would be to use condvar(9).  Generally, tsleep(9) should
> > be replaced and removed in the longer term.
> 
> Sure. Two questions hold me back from doing this:
> 1. Which cv_*wait* variant should be used?
> 2. Where should I add the corresponding cv_signal/cv_broadcast ?
> 
> Is it possible to have an conversion example in the tsleep(9) manpage?

OK, some notes.  The ltsleep(9)/tsleep(9)/mtsleep(9) and wakeup(9) pairs
should generally be replaced by cv_wait*(9) and cv_broadcast(9) pairs, as
the former API is obsolete.

It depends which cv_wait*(9) variant to use.  Simplistic way - look at the
corresponding ltsleep(9) usage.  There are two arguments of interest: timo
and priority.  The priority value may have ORed such flags: PNORELOCK and
PCATCH.  The PCATCH means that blocking thread should be awoken on signal,
which would be done by cv_wait_sig(9).  The timo value, if it is not zero,
indicates how long to sleep - replace it with cv_timedwait(9).  If both
specified, there is cv_timedwait_sig(9), and if none - there is cv_wait(9).
Interlock (mutex) must be held across cv_wait*/cv_broadcast calls, in order
to protect our state. Which means that some old code might require amending
(to remove PNORELOCK or replace simple_lock(9) use) or addition of locking.

That is "simplistic", because converting should not be done mechanically
i.e. "blindly".  Code logic should be understood before changing, and it
may also need to be revisited for the change.  Please also read condvar(9)
and ltsleep(9) manual pages - all the details are well described there.

-- 
Mindaugas