From 3e567afed56b92325150f76c6f36646184d6a650 Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Mon, 27 May 2013 00:59:43 +0200 Subject: [PATCH 1/5] kernel: fix sysexec() error handling compiler problem, sysrendez() busyloop the variables elem and file0 and commited are explicitely set to avoid that they get freed in ther waserror() handlers. but it turns out the compiler optimizes this out as he thinks the variables arent used any further. (the compiler is not aware of the waserror() / longjmp() semantics). rearrange the code to account for this. instead of using a local variable to check for point of no return (commited), we use up->seg[SSEG] to figure it out. for file0 and elem, we just rearrange the code. elem can be checked in the error handler if it was already assigned to up->text, and file0 is just free()'d after the poperror(). remove silly busy loop in sysrendez. it is not needed. dequeueproc() will make sure that the process has come to rest. --- sys/src/9/port/sysproc.c | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/sys/src/9/port/sysproc.c b/sys/src/9/port/sysproc.c index 9c5c8899e..0d0e22661 100644 --- a/sys/src/9/port/sysproc.c +++ b/sys/src/9/port/sysproc.c @@ -226,7 +226,7 @@ sysexec(ulong *arg) char *a, *charp, *args, *file, *file0; char *progarg[sizeof(Exec)/2+1], *elem, progelem[64]; ulong ssize, tstk, nargs, nbytes, n, bssend; - int indir, commit; + int indir; Exec exec; char line[sizeof(Exec)]; Fgrp *f; @@ -234,16 +234,16 @@ sysexec(ulong *arg) ulong magic, text, entry, data, bss; Tos *tos; - commit = 0; indir = 0; elem = nil; validaddr(arg[0], 1, 0); file0 = validnamedup((char*)arg[0], 1); if(waserror()){ free(file0); - free(elem); + if(elem != up->text) + free(elem); /* Disaster after commit */ - if(commit) + if(!up->seg[SSEG]) pexit(up->errstr, 1); nexterror(); } @@ -386,14 +386,9 @@ sysexec(ulong *arg) memmove(charp, *argp++, n); charp += n; } - free(file0); - file0 = nil; /* so waserror() won't free file0 */ - USED(file0); free(up->text); up->text = elem; - elem = nil; /* so waserror() won't free elem */ - USED(elem); /* copy args; easiest from new process's stack */ n = charp - args; @@ -415,9 +410,6 @@ sysexec(ulong *arg) } up->nargs = n; - commit = 1; - USED(commit); - /* * Committed. * Free old memory. @@ -428,9 +420,9 @@ sysexec(ulong *arg) /* prevent a second free if we have an error */ up->seg[i] = 0; } - for(i = BSEG+1; i < NSEG; i++) { + for(i = ESEG+1; i < NSEG; i++) { s = up->seg[i]; - if(s != 0 && (s->type&SG_CEXEC)) { + if(s != 0 && (s->type&SG_CEXEC) != 0) { putseg(s); up->seg[i] = 0; } @@ -439,9 +431,10 @@ sysexec(ulong *arg) /* * Close on exec */ - f = up->fgrp; - for(i=0; i<=f->maxfd; i++) - fdclose(i, CCEXEC); + if((f = up->fgrp) != nil){ + for(i=0; i<=f->maxfd; i++) + fdclose(i, CCEXEC); + } /* Text. Shared. Attaches to cache image if possible */ /* attachimage returns a locked cache image */ @@ -484,9 +477,10 @@ sysexec(ulong *arg) if(devtab[tc->type]->dc == L'/') up->basepri = PriRoot; up->priority = up->basepri; - cclose(tc); poperror(); /* tc */ - poperror(); /* elem */ + cclose(tc); + poperror(); /* file0 */ + free(file0); qlock(&up->debug); up->nnote = 0; @@ -830,9 +824,9 @@ sysrendezvous(ulong *arg) val = p->rendval; p->rendval = arg[1]; unlock(up->rgrp); - while(p->mach != 0) - ; + ready(p); + return val; } l = &p->rendhash; @@ -1140,7 +1134,8 @@ syssemrelease(ulong *arg) if((s = seg(up, (ulong)addr, 0)) == nil) error(Ebadarg); + /* delta == 0 is a no-op, not a release */ if(delta < 0 || *addr < 0) error(Ebadarg); - return semrelease(s, addr, arg[1]); + return semrelease(s, addr, delta); } From 24b908be8a3edb75a63683eb5008aba3c9a1bb52 Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Mon, 27 May 2013 01:04:53 +0200 Subject: [PATCH 2/5] kernel: image reclaim pauses get a bit more verbose about process image exhaustion and make imagreclaim() try to get at least one image on the freelist. use rsrcwait() to notify the state, and call freebroken() in case imagereclaim() couldnt free any images. --- sys/src/9/port/segment.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sys/src/9/port/segment.c b/sys/src/9/port/segment.c index 98e9cbfa0..d3ad7cfbb 100644 --- a/sys/src/9/port/segment.c +++ b/sys/src/9/port/segment.c @@ -264,7 +264,10 @@ attachimage(int type, Chan *c, ulong base, ulong len) while(!(i = imagealloc.free)) { unlock(&imagealloc); imagereclaim(); - sched(); + if(!imagealloc.free){ + freebroken(); /* can use the memory */ + resrcwait("no image after reclaim"); + } lock(&imagealloc); } @@ -328,7 +331,7 @@ imagereclaim(void) * end of the list (see putpage) so start there and work * backward. */ - for(p = palloc.tail; p && p->image && n<1000; p = p->prev) { + for(p = palloc.tail; p && p->image && (n<1000 || !imagealloc.free); p = p->prev) { if(p->ref == 0 && canlock(p)) { if(p->ref == 0) { n++; From c4153b7755e3a4f3cab57ad8cbe85d18cc7cbd77 Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Mon, 27 May 2013 01:09:34 +0200 Subject: [PATCH 3/5] kernel: closechanq error catch potential interrupt error from kproc(). this can happen when we run out of processes, then newproc() will call rsrcwait() which does tsleep(). if the process gets a note, this might raise a interrupt error. --- sys/src/9/port/chan.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sys/src/9/port/chan.c b/sys/src/9/port/chan.c index d0d0ce4bb..9577eb8d3 100644 --- a/sys/src/9/port/chan.c +++ b/sys/src/9/port/chan.c @@ -536,7 +536,10 @@ closechanq(Chan *c) if(up != 0 && palloc.Lock.p != up && canqlock(&clunkq.q)){ c = up->dot; up->dot = nil; - kproc("closeproc", closeproc, nil); + if(!waserror()){ + kproc("closeproc", closeproc, nil); + poperror(); + } up->dot = c; }else wakeup(&clunkq.r); From 410d6bea6ac0ac8d57b6c531b8829152cb503e9d Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Mon, 27 May 2013 01:12:21 +0200 Subject: [PATCH 4/5] devfs/devsd: fix waserror() and unused variable compiler problem the compiler optimizes setting unused variables out, which is problematic if they are used in waserror() handler which the compiler isnt aware of. rearrange the code to avoid this problem. --- sys/src/9/port/devfs.c | 32 +++++++++++++++++++------------- sys/src/9/port/devsd.c | 8 +++----- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/sys/src/9/port/devfs.c b/sys/src/9/port/devfs.c index c5781ee17..a1ee598b4 100644 --- a/sys/src/9/port/devfs.c +++ b/sys/src/9/port/devfs.c @@ -748,29 +748,33 @@ rdconf(void) } else mustrd = 1; - /* read it */ - cc = nil; - c = nil; - if (waserror()){ - if (cc != nil) - cclose(cc); - if (c) - free(c); - if (!mustrd) + if(waserror()){ + if(!mustrd) return; nexterror(); } + + /* read it */ cc = namec(s, Aopen, OREAD, 0); + if(waserror()){ + cclose(cc); + nexterror(); + } devtab[cc->type]->read(cc, confstr, sizeof confstr, 0); + poperror(); cclose(cc); - cc = nil; /* validate, copy and erase config; mconfig will repopulate confstr */ if (strncmp(confstr, cfgstr, sizeof cfgstr - 1) != 0) error("bad #k config, first line must be: 'fsdev:\\n'"); - kstrdup(&c, confstr + sizeof cfgstr - 1); - memset(confstr, 0, sizeof confstr); + c = nil; + kstrdup(&c, confstr + sizeof cfgstr - 1); + if(waserror()){ + free(c); + nexterror(); + } + memset(confstr, 0, sizeof confstr); /* process config copy one line at a time */ for (p = c; p != nil && *p != '\0'; p = e){ e = strchr(p, '\n'); @@ -780,8 +784,10 @@ rdconf(void) e++; mconfig(p, e - p); } - USED(cc); /* until now, can be used in waserror clause */ poperror(); + free(c); + + poperror(); /* mustrd */ } static int diff --git a/sys/src/9/port/devsd.c b/sys/src/9/port/devsd.c index 5cc44b31c..4a11d718c 100644 --- a/sys/src/9/port/devsd.c +++ b/sys/src/9/port/devsd.c @@ -1558,10 +1558,10 @@ sdwstat(Chan* c, uchar* dp, int n) d = nil; if(waserror()){ - free(d); qunlock(&unit->ctl); if(sdev != nil) decref(&sdev->r); + free(d); nexterror(); } @@ -1600,13 +1600,11 @@ sdwstat(Chan* c, uchar* dp, int n) error(Eperm); if(d[0].mode != ~0UL) perm->perm = (perm->perm & ~0777) | (d[0].mode & 0777); - - free(d); - d = nil; USED(d); + poperror(); qunlock(&unit->ctl); if(sdev != nil) decref(&sdev->r); - poperror(); + free(d); return n; } From 44e4aad870a525a0a99c95a18f92bfc71768692f Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Mon, 27 May 2013 01:17:11 +0200 Subject: [PATCH 5/5] kernel: dont copy fpsave on fork, simplify freeing waitq in pexit(), remove unused semlock from Proc sturcure --- sys/src/9/pc/trap.c | 4 ---- sys/src/9/port/portdat.h | 1 - sys/src/9/port/proc.c | 9 ++++----- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/sys/src/9/pc/trap.c b/sys/src/9/pc/trap.c index 1746e963c..a2106df45 100644 --- a/sys/src/9/pc/trap.c +++ b/sys/src/9/pc/trap.c @@ -734,10 +734,6 @@ syscall(Ureg* ureg) scallnr = ureg->ax; up->scallnr = scallnr; - if(scallnr == RFORK && up->fpstate == FPactive){ - fpsave(&up->fpsave); - up->fpstate = FPinactive; - } spllo(); up->nerrlab = 0; diff --git a/sys/src/9/port/portdat.h b/sys/src/9/port/portdat.h index 92e5af666..7ead18a00 100644 --- a/sys/src/9/port/portdat.h +++ b/sys/src/9/port/portdat.h @@ -421,7 +421,6 @@ struct Segment Pte **map; int mapsize; Pte *ssegmap[SSEGMAPSIZE]; - Lock semalock; Sema sema; ulong mark; /* portcountrefs */ }; diff --git a/sys/src/9/port/proc.c b/sys/src/9/port/proc.c index 348ca1592..bbceff5ef 100644 --- a/sys/src/9/port/proc.c +++ b/sys/src/9/port/proc.c @@ -1067,7 +1067,7 @@ pexit(char *exitstr, int freemem) Proc *p; Segment **s, **es; long utime, stime; - Waitq *wq, *f, *next; + Waitq *wq; Fgrp *fgrp; Egrp *egrp; Rgrp *rgrp; @@ -1178,9 +1178,9 @@ pexit(char *exitstr, int freemem) wakeup(&up->waitr); unlock(&up->exl); - for(f = up->waitq; f; f = next) { - next = f->next; - free(f); + while((wq = up->waitq) != 0){ + up->waitq = wq->next; + free(wq); } /* release debuggers */ @@ -1374,7 +1374,6 @@ kproc(char *name, void (*func)(void *), void *arg) p->kp = 1; p->noswap = 1; - p->fpsave = up->fpsave; p->scallnr = up->scallnr; p->s = up->s; p->nerrlab = 0;