* calling sleep() while holding lock()
@ 1997-05-08 14:04 G.David
0 siblings, 0 replies; 7+ messages in thread
From: G.David @ 1997-05-08 14:04 UTC (permalink / raw)
It would seem to me that a process should not call sleep() holding
a spinlock, even though that seems to be happening.
I changed taslock to increment and decrement the hasspin flag instead of just
setting it and clearing it. It is reasonable to have many locks.
(There is also a problem with the lock being dropped before the hasspin
was modified that I fixed. I also temporarly removed the hasspin clear
from clock.c)
I then added a print in sleep to print the pid and hasspin counter if
hasspin > 0. It happens alot and pretty early in the boot phase.
I'm doing this trying to find the cause of my earlier message about
checksum errors on the ethernet. I am looking for places where spinlocks
are being held for long times and next where interrupts are masked
too long.
Before I go much further, I wanted to check on this behavior.
Thanks for any info.
David Butler
gdb@dbSystems.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-09 16:16 Paul
0 siblings, 0 replies; 7+ messages in thread
From: Paul @ 1997-05-09 16:16 UTC (permalink / raw)
> From: Lucio de Re <lucio@proxima.alt.za>
>
> Very well formulated. The license restrictions remain a stumbling block
> but it would be nice to ask Lucent to change them, if we could decide what
> it is that we all want :-)
Have you asked them? I believe the license agreement states that you
should. As an anecdote, I wanted PGP but did not want to risk litigation.
I grumbled, as everyone did, that the only way to get it was through
ViaCrypt, which must be way too expensive. Finally I called ViaCrypt and
asked about it. It turned out that instead of the thousdands of dollars
I expected to hear, it was only $149. When you look at it as a one time
insurance premium it does not look so bad! Anyhow, contact Lucent first
and see what happens. I don't know what will. Maybe they will surprise
you (and hopefully in a good way!)
> My two cents' worth is that I'd like to use Plan 9 commercially, although
> in a very restricted environment: I operate as a low key ISP, and I'd like
> all my computers to be served from a Plan 9 core network - one or two
> clients would similarly benefit from running Plan 9 instead of Windows NT
> as a core server.
I certainly hope you don't think if you fail to get licensing terms from
Lucent that your only other option is NT! Several other options spring
to mind without even thinking about it.
-Paul Borman
prb@bsdi.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-09 13:41 G.David
0 siblings, 0 replies; 7+ messages in thread
From: G.David @ 1997-05-09 13:41 UTC (permalink / raw)
I wrote:
>The boddles follow relative to /sys/src/9 with the original files
>called file_name.cdrom (e.g. port/fault.c.cdrom)
>
>To make sure you get the whole thing, the last line says ThEeNd.
>Remove it before you send it to rc -X.
Well some mailer along the way doesn't like lines with a lone period :-)
Take the following and do a 1,$s/^.// on it from ed and it will
be usable
David Butler
gdb@dbSystems.com
##!/bin/rc
##
## command: /bin/boddle pc/clock.c.cdrom pc/clock.c
## srcdir: pc
## version: 863162201
## date: Fri May 9 02:16:41 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162201
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=pc
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162201 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162201
#mkdir 863162201
#
#target=863162201/clock.c.cdrom
#echo -n '863162201/clock.c.cdrom: '
#if(! test -f $srcdir/clock.c.cdrom || ! test -r $srcdir/clock.c.cdrom){
# echo $srcdir/clock.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/clock.c.cdrom}
#if(! ~ a11baa684391 $sum(1)^$sum(2)){
# echo $srcdir/clock.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/clock.c.cdrom 863162201/clock.c.cdrom
#ed 863162201/clock.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM clock.c.cdrom'
#66,70c
# if(p->hasspin == 0 && anyready()){
# sched();
#.
#62c
# if(u && p && p->state == Running){
#.
#wq
#//GO.SYSIN DD VADIM clock.c.cdrom
#sum=`{sum < 863162201/clock.c.cdrom}
#if(~ b4c370264365 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162201/clock.c.cdrom checksum error creating updated file
# exit checksum
#}
##!/bin/rc
##
## command: /bin/boddle port/devbit.c.cdrom port/devbit.c
## srcdir: port
## version: 863162229
## date: Fri May 9 02:17:09 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162229
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=port
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162229 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162229
#mkdir 863162229
#
#target=863162229/devbit.c.cdrom
#echo -n '863162229/devbit.c.cdrom: '
#if(! test -f $srcdir/devbit.c.cdrom || ! test -r $srcdir/devbit.c.cdrom){
# echo $srcdir/devbit.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/devbit.c.cdrom}
#if(! ~ d3657f7546179 $sum(1)^$sum(2)){
# echo $srcdir/devbit.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/devbit.c.cdrom 863162229/devbit.c.cdrom
#ed 863162229/devbit.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM devbit.c.cdrom'
#432d
#429a
# free(b);
#.
#426a
# b = smalloc(sizeof(GBitmap));
#.
#wq
#//GO.SYSIN DD VADIM devbit.c.cdrom
#sum=`{sum < 863162229/devbit.c.cdrom}
#if(~ 45a5486b46191 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162229/devbit.c.cdrom checksum error creating updated file
# exit checksum
#}
##!/bin/rc
##
## command: /bin/boddle port/fault.c.cdrom port/fault.c
## srcdir: port
## version: 863162247
## date: Fri May 9 02:17:27 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162247
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=port
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162247 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162247
#mkdir 863162247
#
#target=863162247/fault.c.cdrom
#echo -n '863162247/fault.c.cdrom: '
#if(! test -f $srcdir/fault.c.cdrom || ! test -r $srcdir/fault.c.cdrom){
# echo $srcdir/fault.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/fault.c.cdrom}
#if(! ~ 2a5f95575928 $sum(1)^$sum(2)){
# echo $srcdir/fault.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/fault.c.cdrom 863162247/fault.c.cdrom
#ed 863162247/fault.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM fault.c.cdrom'
#144c
# if ((new = malloc(sizeof(Page))) == 0)
# panic("malloc in fixfault");
#.
#wq
#//GO.SYSIN DD VADIM fault.c.cdrom
#sum=`{sum < 863162247/fault.c.cdrom}
#if(~ 9c3feeeb5972 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162247/fault.c.cdrom checksum error creating updated file
# exit checksum
#}
##!/bin/rc
##
## command: /bin/boddle port/proc.c.cdrom port/proc.c
## srcdir: port
## version: 863162273
## date: Fri May 9 02:17:53 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162273
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=port
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162273 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162273
#mkdir 863162273
#
#target=863162273/proc.c.cdrom
#echo -n '863162273/proc.c.cdrom: '
#if(! test -f $srcdir/proc.c.cdrom || ! test -r $srcdir/proc.c.cdrom){
# echo $srcdir/proc.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/proc.c.cdrom}
#if(! ~ d4143c7915168 $sum(1)^$sum(2)){
# echo $srcdir/proc.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/proc.c.cdrom 863162273/proc.c.cdrom
#ed 863162273/proc.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM proc.c.cdrom'
#693a
# unlock(&p->exl);
#.
#691d
#542a
# unlock(&p->rlock);
# splx(s);
#.
#527,541c
# s = splhi();
# lock(&p->rlock);
# r = p->r;
# if(r){
# if(r->p != p || p->r != r || p->state != Wakeme)
# panic("postnote: state");
# r->p = 0;
# p->r = 0;
# ready(p);
#.
#479c
#
# unlock(&p->rlock);
#.
#472c
# if(p == 0)
# return;
#
# s = splhi();
# lock(&p->rlock);
#
# if(r->p == p && p->r == r){
#.
#469,470d
#406c
# unlock(&p->rlock);
#.
#403c
# lock(&p->rlock);
#.
#396a
# if (p->hasspin)
# panic("hasspin %d %d\n", p->pid, p->hasspin);
#.
#386,387c
# p->r = r;
# unlock(&p->rlock);
#.
#381,384d
#371,372c
# r->p = 0;
# unlock(&p->rlock);
#.
#364,365c
# s = splhi();
# lock(&p->rlock);
# if(r->p){
# print("double sleep %d %d\n", r->p->pid, p->pid);
# dumpstack();
# }
# r->p = p;
#.
#362d
#352c
#static void
#.
#322c
# p->mp = m;
#.
#308a
# p->hasspin = 0;
#.
#94a
# p = u->p;
# if (p->pid && p->hasspin)
# panic("hasspin %d %d\n", p->pid, p->hasspin);
#.
#wq
#//GO.SYSIN DD VADIM proc.c.cdrom
#sum=`{sum < 863162273/proc.c.cdrom}
#if(~ d8988e3715301 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162273/proc.c.cdrom checksum error creating updated file
# exit checksum
#}
##!/bin/rc
##
## command: /bin/boddle port/segment.c.cdrom port/segment.c
## srcdir: port
## version: 863162294
## date: Fri May 9 02:18:14 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162294
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=port
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162294 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162294
#mkdir 863162294
#
#target=863162294/segment.c.cdrom
#echo -n '863162294/segment.c.cdrom: '
#if(! test -f $srcdir/segment.c.cdrom || ! test -r $srcdir/segment.c.cdrom){
# echo $srcdir/segment.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/segment.c.cdrom}
#if(! ~ 7c34b5a48705 $sum(1)^$sum(2)){
# echo $srcdir/segment.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/segment.c.cdrom 863162294/segment.c.cdrom
#ed 863162294/segment.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM segment.c.cdrom'
#228c
# sched();
#.
#90a
# unlock(s);
#.
#72,73d
#67d
#63c
# lock(s);
#.
#wq
#//GO.SYSIN DD VADIM segment.c.cdrom
#sum=`{sum < 863162294/segment.c.cdrom}
#if(~ 3203b05a8693 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162294/segment.c.cdrom checksum error creating updated file
# exit checksum
#}
##!/bin/rc
##
## command: /bin/boddle port/taslock.c.cdrom port/taslock.c
## srcdir: port
## version: 863162313
## date: Fri May 9 02:18:33 CDT 1997
##
#myname=$0
#doextract=no
#
#fn usage{
# echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
# exit usage
#}
#
#fn sigint{
# rm -rf 863162313
# exit interrupt
#}
#
#while(~ $1 -*){
# switch($1){
# case -X
# doextract=yes
# case -*
# usage
# }
# shift
#}
#
#switch($#*){
#case 0
# srcdir=port
#case 1
# srcdir=$1
#case *
# usage
#}
#
#if(! ~ $doextract yes){
# echo This shell file contains a bundle of diffs representing changes
# echo to original source files in the Plan 9 distribution. It will run
# echo against the files in
# echo ' ' $srcdir
# echo '(unless overridden by the optional source directory argument)'
# echo and create a directory 863162313 containing the updated files.
# echo It will NOT automatically update the original files.
# echo
# echo Invoke with argument -X to perform the actual extraction.
# exit 0
#}
#
#rm -rf 863162313
#mkdir 863162313
#
#target=863162313/taslock.c.cdrom
#echo -n '863162313/taslock.c.cdrom: '
#if(! test -f $srcdir/taslock.c.cdrom || ! test -r $srcdir/taslock.c.cdrom){
# echo $srcdir/taslock.c.cdrom unreadable
# exit unreadable
#}
#sum=`{sum < $srcdir/taslock.c.cdrom}
#if(! ~ 744969121109 $sum(1)^$sum(2)){
# echo $srcdir/taslock.c.cdrom is not the original distribution file
# exit original
#}
#cp $srcdir/taslock.c.cdrom 863162313/taslock.c.cdrom
#ed 863162313/taslock.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM taslock.c.cdrom'
#76a
# if(u && u->p)
# (u->p->hasspin)--;
#.
#68,69d
#65a
# if(u && u->p)
# (u->p->hasspin)--;
#.
#59c
# (u->p->hasspin)++;
#.
#48,49c
# panic("lock loop 0x%lux pc 0x%lux held by pc 0x%lux\n",
# l, pc, l->pc);
#.
#42a
# if(u && u->p)
# (u->p->hasspin)++;
#.
#25,26c
# panic("lock loop 0x%lux pc 0x%lux held by pc 0x%lux\n",
# l, pc, l->pc);
#.
#20a
# if(u && u->p)
# (u->p->hasspin)++;
#.
#18,19d
#wq
#//GO.SYSIN DD VADIM taslock.c.cdrom
#sum=`{sum < 863162313/taslock.c.cdrom}
#if(~ f7c98ba41155 $sum(1)^$sum(2))
# echo
#if not{
# echo 863162313/taslock.c.cdrom checksum error creating updated file
# exit checksum
#}
ThEeNd
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-09 8:00 Lucio
0 siblings, 0 replies; 7+ messages in thread
From: Lucio @ 1997-05-09 8:00 UTC (permalink / raw)
>
> BTW: THIS is the future of Plan9. What ever we make of it.
>
> David Butler
> gdb@dbSystems.com
Very well formulated. The license restrictions remain a stumbling block but it would be nice to ask Lucent to change them, if we could decide what it is that we all want :-)
My two cents' worth is that I'd like to use Plan 9 commercially, although in a very restricted environment: I operate as a low key ISP, and I'd like all my computers to be served from a Plan 9 core network - one or two clients would similarly benefit from running Plan 9 instead of Windows NT as a core server.
Then again, I may not be representative, although it would be nice to think that there are affinities with others on this list.
--
Lucio de Re (lucio@proxima.alt.za)
Disclaimer: I'm working at getting my opinions to agree with me.
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-09 7:22 G.David
0 siblings, 0 replies; 7+ messages in thread
From: G.David @ 1997-05-09 7:22 UTC (permalink / raw)
Thanks for the input!
After having the stuff scared out of me with the mail from ncube, I
started to see just how bad it was. I have not finished by any means,
but I now have a system not sleeping on spin locks!
A summary of the changes:
port/devbit.c
move the smalloc outside the lock adding a free if necessary.
port/fault.c
change the smalloc to a malloc with a panic if it fails.
smalloc is the "sleeping" malloc. malloc does not sleep,
it returns 0 if there is no memory.
port/proc.c
added a zero assignment to hasspin on newproc()
added a panic if hasspin is set for the process sleeping.
added a panic if hasspin is set for the process in sched.
port/taslock.c
changed flagging hasspin to increment and decrement.
port/segment.c
added a missing unlock (found by adding a "stack" to lock and
unlock that is not included.)
pc/clock.c (All my machines are PCs)
removed the zero of hasspin.
The boddles follow relative to /sys/src/9 with the original files
called file_name.cdrom (e.g. port/fault.c.cdrom)
To make sure you get the whole thing, the last line says ThEeNd.
Remove it before you send it to rc -X.
Let me know how it goes.
BTW: THIS is the future of Plan9. What ever we make of it.
David Butler
gdb@dbSystems.com
#!/bin/rc
#
# command: /bin/boddle pc/clock.c.cdrom pc/clock.c
# srcdir: pc
# version: 863162201
# date: Fri May 9 02:16:41 CDT 1997
#
myname=$0
doextract=no
fn usage{
echo $myname: usage: $myname '[-X] [src-directory]' >[1=2]
exit usage
}
fn sigint{
rm -rf 863162201
exit interrupt
}
while(~ $1 -*){
switch($1){
case -X
doextract=yes
case -*
usage
}
shift
}
switch($#*){
case 0
srcdir=pc
case 1
srcdir=$1
case *
usage
}
if(! ~ $doextract yes){
echo This shell file contains a bundle of diffs representing changes
echo to original source files in the Plan 9 distribution. It will run
echo against the files in
echo ' ' $srcdir
echo '(unless overridden by the optional source directory argument)'
echo and create a directory 863162201 containing the updated files.
echo It will NOT automatically update the original files.
echo
echo Invoke with argument -X to perform the actual extraction.
exit 0
}
rm -rf 863162201
mkdir 863162201
target=863162201/clock.c.cdrom
echo -n '863162201/clock.c.cdrom: '
if(! test -f $srcdir/clock.c.cdrom || ! test -r $srcdir/clock.c.cdrom){
echo $srcdir/clock.c.cdrom unreadable
exit unreadable
}
sum=`{sum < $srcdir/clock.c.cdrom}
if(! ~ a11baa684391 $sum(1)^$sum(2)){
echo $srcdir/clock.c.cdrom is not the original distribution file
exit original
}
cp $srcdir/clock.c.cdrom 863162201/clock.c.cdrom
ed 863162201/clock.c.cdrom >/dev/null >[2=1] <<'//GO.SYSIN DD VADIM clock.c.cdrom'
66,70c
if(p->hasspin == 0 && anyready()){
sched();
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-08 18:32 beto
0 siblings, 0 replies; 7+ messages in thread
From: beto @ 1997-05-08 18:32 UTC (permalink / raw)
In <199705090253.496031.out.bajag@plan9.cs.su.oz.au>
David Hogan <dhog@lore.plan9.cs.su.oz.au> wrote:
> Good coding. So now the question remains: why is this behaviour
> occuring? One possibility is that we take a fault while holding
> the lock, and we then have to sleep until the memory gets paged in.
> Alberto Nava found a place in the kernel where this is happening, and
> I'm sure there must be others.
>
Here is the example that Dave mentioned:
long
bitread(Chan *c, void *va, long n, ulong offset)
{
uchar *p, *q;
long miny, maxy, t, x, y;
ulong l, nw, ws, rv, gv, bv;
int off, j;
Fontchar *i;
GBitmap *src;
BSubfont *s;
static int map[8] = {0, 4, 2, 6, 1, 5, 3, 7 };
if(!conf.monitor)
error(Egreg);
if(c->qid.path & CHDIR)
return devdirread(c, va, n, bitdir, NBIT, devgen);
if(c->qid.path == Qmouse){
/*
* mouse:
* 'm' 1
* buttons 1
* point 8
* msec 4
*/
if(n < 14)
error(Ebadblt);
while(mousechanged(0) == 0)
sleep(&mouse.r, mousechanged, 0);
lock(&cursor);
p = va; ***************************************
Accessing va could generate a page-fault. This could lead to
seg() calling qlock or sleeping waiting for the actual data.
There are plenty of places where plan9 does that or similar. For example
in the same driver:
Chan*
bitopen(Chan *c, int omode)
{
GBitmap *b;
if(!conf.monitor)
error(Egreg);
switch(c->qid.path){
case CHDIR:
if(omode != OREAD)
error(Eperm);
break;
case Qmouse:
lock(&bit);
if(bit.mouseopen){
unlock(&bit);
error(Einuse);
}
bit.mouseopen = 1;
bit.ref++;
unlock(&bit);
break;
case Qbitblt:
lock(&bit);
if(bit.bitbltopen || bit.mouseopen){
unlock(&bit);
error(Einuse);
}
b = smalloc(sizeof(GBitmap));***************************************
smalloc could make the kernel sleep waiting for memory so again
holding spin-lock/sleep.
Actually we have code in qlock and sleep to check for a process holding
spin-lock before the calls.
^ permalink raw reply [flat|nested] 7+ messages in thread
* calling sleep() while holding lock()
@ 1997-05-08 16:53 David
0 siblings, 0 replies; 7+ messages in thread
From: David @ 1997-05-08 16:53 UTC (permalink / raw)
David Butler wrote:
> It would seem to me that a process should not call sleep() holding
> a spinlock, even though that seems to be happening.
This seems reasonable to me.
> I changed taslock to increment and decrement the hasspin flag instead of just
> setting it and clearing it. It is reasonable to have many locks.
> (There is also a problem with the lock being dropped before the hasspin
> was modified that I fixed. I also temporarly removed the hasspin clear
> from clock.c)
> I then added a print in sleep to print the pid and hasspin counter if
> hasspin > 0. It happens alot and pretty early in the boot phase.
Good coding. So now the question remains: why is this behaviour
occuring? One possibility is that we take a fault while holding
the lock, and we then have to sleep until the memory gets paged in.
Alberto Nava found a place in the kernel where this is happening, and
I'm sure there must be others.
It's not that good to take a fault while holding a spinlock; at
minimum, there will be a loss of efficiency. In the worst case,
the kernel may deadlock or panic. Code which allows this to happen
should be tracked down, and changed to either use a local buffer
in the critical section, or else verify that the memory is writable
first...
You should add another print to the fault handler, so that you can
see which of the sleeps are caused by faults, and which aren't.
You might want to record the caller PC of the most recent spinlock,
and print that as well. This will enable locating which parts of
the kernel are behaving this way.
When you've got a list of PC values, use acid to find the file &
line number for each, and post them! I for one would be interested
in this data.
> I'm doing this trying to find the cause of my earlier message about
> checksum errors on the ethernet. I am looking for places where spinlocks
> are being held for long times and next where interrupts are masked
> too long.
I've noticed that the Plan 9 kernel does go through some quite long
periods at high IPL. During these, it is possible to lose serial
characters at a mere 9600 baud :-( Any insight into why this
happens would be appreciated. I was going to add some code to the
kernel to keep a journaling buffer of (PC, microsecond) pairs recorded
at strategic points in the kernel (such as every call to splhi &co),
but I never got around to it. I may yet do this, now that I have
a decent machine at home and spare time on weekends...
> Before I go much further, I wanted to check on this behavior.
The less time spent holding spinlocks, the better. Your mission,
if you choose to accept it, is to obtain the release of the lost
CPU cycles. If you are caught, Dennis will disavow all knowledge
of your actions.
> Thanks for any info.
You're welcome.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~1997-05-09 16:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1997-05-08 14:04 calling sleep() while holding lock() G.David
1997-05-08 16:53 David
1997-05-08 18:32 beto
1997-05-09 7:22 G.David
1997-05-09 8:00 Lucio
1997-05-09 13:41 G.David
1997-05-09 16:16 Paul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).