NetBSD Problem Report #48671

From tsutsui@ceres.dti.ne.jp  Fri Mar 21 08:23:53 2014
Return-Path: <tsutsui@ceres.dti.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 8E819A5805
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 21 Mar 2014 08:23:53 +0000 (UTC)
Message-Id: <201403210823.s2L8NnhR003063@mirage.localdomain>
Date: Fri, 21 Mar 2014 17:23:49 +0900 (JST)
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Reply-To: tsutsui@ceres.dti.ne.jp
To: gnats-bugs@gnats.NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Add description how to convert from tsleep(9)/wakeup(9) to condvar(9)
X-Send-Pr-Version: 3.95

>Number:         48671
>Category:       misc
>Synopsis:       Add description how to convert from tsleep(9)/wakeup(9) to condvar(9)
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    misc-bug-people
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 21 08:25:00 +0000 2014
>Closed-Date:    Tue May 31 06:38:01 +0000 2016
>Last-Modified:  Tue May 31 06:38:01 +0000 2016
>Originator:     Izumi Tsutsui
>Release:        NetBSD 6.1.3
>Organization:
>Environment:
System: NetBSD 6.1.3
>Description:
rmind@ posted "converting to condvar(9)" message on tech-kern:
http://mail-index.netbsd.org/tech-kern/2010/09/05/msg008831.html
It would be worth to also mention it in deprecated tsleep(9)/wakeup(9)
man page to nuke them in future release.
>How-To-Repeat:
man tsleep

Currently it just says:
>> The interfaces described in this manual page are obsolete and will be
>> removed from a future version of the system.
>> 
>> The ltsleep() interface has been obsoleted and removed from the system.
>> 
>> Please see the condvar(9), mutex(9), and rwlock(9) manual pages for
>> information on kernel synchronisation primitives.

>Fix:
Here is a dumb patch which adds "MIGRATING TO CONDVAR" section
to ltsleep(9) per his description. (needs more proofreadings though)

Index: ltsleep.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/ltsleep.9,v
retrieving revision 1.14
diff -u -p -d -r1.14 ltsleep.9
--- ltsleep.9	28 Jan 2012 13:26:12 -0000	1.14
+++ ltsleep.9	21 Mar 2014 08:01:24 -0000
@@ -166,6 +166,80 @@ If
 .Fn tsleep
 returns because of a timeout it returns
 .Er EWOULDBLOCK .
+.Sh MIGRATING TO CONDVAR
+Note the conversion from tsleep/wakeup into
+.Xr condvar 9
+should not be done mechanically i.e.
+.Dq blindly .
+Code logic should be understood before changing, and it may also need to be
+revisited for the change.
+Please also read
+.Xr condvar 9
+man page.
+.Pp
+The
+.Fn tsleep
+and
+.Fn wakeup
+pairs should generally be replaced by
+.Xr cv_wait 9 /
+.Xr cv_wait_sig 9 /
+.Xr cv_timedwait 9 /
+.Xr cv_timedwait_sig 9
+and
+.Xr cv_signal 9 /
+.Xr cv_broadcast 9
+pairs.
+It depends which
+.Fn cv_wait*
+variant to use, and it would be guessed by looking at the corresponding
+.Fn tsleep
+usage.
+There are two arguments of interest:
+.Ar timo
+and
+.Ar priority .
+The
+.Ar priority
+value may have OR'ed such flags:
+.Dv PNORELOCK
+and
+.Dv PCATCH .
+.Pp
+The
+.Dv PCATCH
+means that blocking thread should be awoken on signal
+and it would be done by
+.Xr cv_wait_sig 9 .
+.Pp
+The
+.Ar timo
+value, if it is not zero, indicates how long to sleep and
+it would be done by
+.Xr cv_timedwait 9 .
+.Pp
+If both
+.Dv PCACHE
+and
+non-zero
+.Ar timo
+value are specified, there is
+.Xr cv_timedwait_sig 9 .
+.Pp
+If none is specified, there is
+.Xr cv_wait 9 .
+.Pp
+Interlock (mutex) must be held across
+.Fn cv_wait*
+and
+.Fn cv_broadcast
+calls, in order to protect our state.
+Which means that some old code might require amending
+(to remove
+.Dv PNORELOCK
+or replace
+.Xr simple_lock 9
+use) or addition of locking.
 .Sh SEE ALSO
 .Xr sigaction 2 ,
 .Xr condvar 9 ,

>Release-Note:

>Audit-Trail:
From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>, misc-bug-people@netbsd.org, 
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: misc/48671: Add description how to convert from tsleep(9)/wakeup(9)
 to condvar(9)
Date: Fri, 21 Mar 2014 16:55:09 +0000

 This is a multi-part message in MIME format.
 --------------030601020508010607040902
 Content-Type: text/plain; charset=ISO-8859-1; format=flowed
 Content-Transfer-Encoding: 7bit

 On 03/21/14 08:25, Izumi Tsutsui wrote:
 >> Number:         48671
 >> Category:       misc
 >> Synopsis:       Add description how to convert from tsleep(9)/wakeup(9) to condvar(9)
 >>
 Good idea.

 I've updated your patch a little.

 Nick

 --------------030601020508010607040902
 Content-Type: text/plain; charset=us-ascii;
  name="ltsleep.9.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="ltsleep.9.diff"

 Index: share/man/man9/ltsleep.9
 ===================================================================
 RCS file: /cvsroot/src/share/man/man9/ltsleep.9,v
 retrieving revision 1.14
 diff -u -p -r1.14 ltsleep.9
 --- share/man/man9/ltsleep.9	28 Jan 2012 13:26:12 -0000	1.14
 +++ share/man/man9/ltsleep.9	21 Mar 2014 16:29:53 -0000
 @@ -32,12 +32,15 @@
  .Os
  .Sh NAME
  .Nm ltsleep ,
 +.Nm mtsleep ,
  .Nm tsleep ,
  .Nm wakeup
  .Nd process context sleep and wakeup
  .Sh SYNOPSIS
  .In sys/proc.h
  .Ft int
 +.Fn "mtsleep" "wchan_t ident" "pri_t priority" "const char *wmesg" "int timo" "kmutex_t *mtx"
 +.Ft int
  .Fn "tsleep" "wchan_t ident" "pri_t priority" "const char *wmesg" "int timo"
  .Ft void
  .Fn "wakeup" "wchan_t ident"
 @@ -58,7 +61,9 @@
  .Pp
  These functions implement voluntary context switching.
  .Fn tsleep
 -is used throughout the kernel whenever processing in the current context
 +and
 +.Fn mtsleep
 +are used throughout the kernel whenever processing in the current context
  can not continue for any of the following reasons:
  .Bl -bullet -offset indent
  .It
 @@ -67,9 +72,6 @@ The current process needs to await the r
  The current process needs resources
  .Pq e.g., memory
  which are temporarily unavailable.
 -.It
 -The current process wants access to data-structures which are locked by
 -other processes.
  .El
  .Pp
  The function
 @@ -83,7 +85,9 @@ condition has cleared.
  .Pp
  The
  .Fn tsleep
 -function takes the following arguments:
 +and
 +.Fn mtsleep
 +functions take the following arguments:
  .Bl -tag -width priority
  .It Fa ident
  An identifier of the
 @@ -129,31 +133,52 @@ will return
  .El
  .Pp
  The
 +.Fn mtsleep
 +function takes an additional argument and flag:
 +.Bl -tag -width priority
 +.It Fa mtx
 +A
 +.Xr mutex 9
 +representing the lock protecting the data-structures. On entry
 +.Fn mtsleep
 +will release the lock and re-acquire the lock on return.
 +.It Fa priority
 +If the flag
 +.Dv PNORELOCK
 +is OR'ed into
 +.Fa priority
 +then
 +.Fn mtsleep
 +will not re-acquire the lock.
 +.El
 +.Pp
 +The
  .Fn wakeup
  function will mark all processes which are currently sleeping on the identifier
  .Fa ident
  as runnable.
  Eventually, each of the processes will resume execution in the kernel
  context, causing a return from
 -.Fn tsleep .
 +.Fn tsleep
 +or
 +.Fn mtsleep .
  Note that processes returning from sleep should always re-evaluate the
  conditions that blocked them, since a call to
  .Fn wakeup
  merely signals a
  .Em possible
  change to the blocking conditions.
 -For example, when two or more processes are waiting for an exclusive-access
 -lock
 -.Pq see Xr lock 9 ,
 -only one of them will succeed in acquiring the lock when it is released.
 -All others will have to go back to sleep and wait for the next opportunity.
  .Sh RETURN VALUES
  .Fn tsleep
 -returns 0 if it returns as a result of a
 +and
 +.Fn mtsleep
 +return 0 if they return as a result of a
  .Fn wakeup .
  If a
  .Fn tsleep
 -returns as a result of a signal, the return value is
 +and
 +.Fn mtsleep
 +return as a result of a signal, the return value is
  .Er ERESTART
  if the signal has the
  .Dv SA_RESTART
 @@ -164,13 +189,85 @@ and
  otherwise.
  If
  .Fn tsleep
 +and
 +.Fn mtsleep
  returns because of a timeout it returns
  .Er EWOULDBLOCK .
 +.Sh MIGRATING TO CONDVAR
 +Note the conversion from tsleep/wakeup into
 +.Xr condvar 9
 +should not be done mechanically i.e.
 +.Dq blindly .
 +Code logic should be understood before changing, and it may also need to be
 +revisited for the change.
 +Please also read
 +.Xr condvar 9
 +man page.
 +.Pp
 +The
 +.Fn tsleep
 +and
 +.Fn mtsleep ,
 +and
 +.Fn wakeup
 +pairs should generally be replaced by
 +.Xr cv_wait 9 /
 +.Xr cv_wait_sig 9 /
 +.Xr cv_timedwait 9 /
 +.Xr cv_timedwait_sig 9
 +and
 +.Xr cv_signal 9 /
 +.Xr cv_broadcast 9
 +pairs.
 +The
 +.Fn cv_wait*
 +variant to use can be determinded from looking at the corresponding
 +.Fn tsleep
 +usage.
 +.Pp
 +There are two arguments of interest:
 +.Ar timo
 +and
 +.Ar priority .
 +The
 +.Ar priority
 +value may have OR'ed the flag
 +.Dv PCATCH .
 +.Pp
 +The
 +.Dv PCATCH
 +flag means that the blocking thread should be awoken on signal, and
 +the sleep call should be replaced with
 +.Xr cv_wait_sig 9 .
 +.Pp
 +The
 +.Ar timo
 +value, if it is not zero, indicates how long to sleep, and
 +the sleep call should be replaced with
 +.Xr cv_timedwait 9 .
 +.Pp
 +If both the
 +.Dv PCATCH
 +flag and a non-zero
 +.Ar timo
 +value are specified, then
 +.Xr cv_timedwait_sig 9
 +should be used.
 +.Pp
 +A
 +.Xr mutex 9
 +(interlock) must be held across
 +.Fn cv_wait
 +and
 +.Fn cv_broadcast
 +calls, in order to protect state.
 +Most old code will require the addition of locking, whereas some will
 +require amending to remove
 +.Dv PNORELOCK .
  .Sh SEE ALSO
  .Xr sigaction 2 ,
  .Xr condvar 9 ,
  .Xr hz 9 ,
 -.Xr lock 9 ,
  .Xr mutex 9 ,
  .Xr rwlock 9
  .Sh HISTORY


 --------------030601020508010607040902--

From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: skrll@netbsd.org
Cc: gnats-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
Subject: Re: misc/48671: Add description how to convert from tsleep(9)/wakeup(9)to
	 condvar(9)
Date: Sat, 22 Mar 2014 04:05:46 +0900

 > I've updated your patch a little.

 I think mtsleep(9) addition (and lock(9) xref removal) should be in
 a separate commit. (it also requires new entries in Makefile and set list)
 ---
 Izumi Tsutsui

State-Changed-From-To: open->closed
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Tue, 31 May 2016 06:38:01 +0000
State-Changed-Why:
The requested documentation changes have already been made.


>Unformatted:

Home	PR Database Search