ref: 8d435d074683b347eec263504986af2afa471276
parent: c910ac83167f6a40fdab053c6fdafd4bee83f811
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Jun 8 14:22:56 EDT 2025
kernel: improve imagecache locking The assumptions are the following: - attachimage() is called often (for each exec), needing to acquire the imagealloc lock to walk the list of active images. - putimage() with a Image.ref dropping to 0 is rare, only occuring when the system is under memory pressure and we'r reclaiming pages from the image cache. We used to acquire Image lock while holding imagealloc lock as this was the only way to make sure we do not race with putimage(). We want to avoid contending on the imagealloc lock here, so that attachimage() can make progress and new processes can be execed. So the lock order is now changed, and we use Image.ref to make sure the Image entry isnt freed after releasing imagealloc lock. This now means that putimage() must acquire imagealloc lock and re-check that Image.ref == 0 before moving the entry to the freelist. (holding allocimage lock is the only way to make sure the Image isnt grabbed out of the cache and used). And attachimage() never holds imagealloc lock and Image lock at the same time. In addition, simplify allocimage() call and move the code attaching the first text segment to sysexec(). Recheck the active image list if we had to run imagereclaim() to avoid duplicates. Make attachimage() return a Image* with a unconditional reference so the call is symmetic with putimage().
--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -9,7 +9,7 @@
Block* allocb(int);
int anyhigher(void);
int anyready(void);
-Image* attachimage(int, Chan*, uintptr, ulong);
+Image* attachimage(Chan*);
ulong beswal(ulong);
uvlong beswav(uvlong);
int blocklen(Block*);
--- a/sys/src/9/port/segment.c
+++ b/sys/src/9/port/segment.c
@@ -93,21 +93,16 @@
Pte **pte, **emap;
Image *i;
- if(s == nil)
+ if(s == nil || decref(s) != 0)
return;
i = s->image;
if(i != nil) {lock(i);
- if(decref(s) != 0){- unlock(i);
- return;
- }
if(i->s == s)
i->s = nil;
putimage(i);
- } else if(decref(s) != 0)
- return;
+ }
assert(s->sema.prev == &s->sema);
assert(s->sema.next == &s->sema);
@@ -240,13 +235,11 @@
}
Image*
-attachimage(int type, Chan *c, uintptr base, ulong len)
+attachimage(Chan *c)
{Image *i, **l;
- c->flag &= ~CCACHE;
- cclunk(c);
-
+retry:
lock(&imagealloc);
/*
@@ -253,28 +246,22 @@
* Search the image cache for remains of the text from a previous
* or currently running incarnation
*/
- for(i = ihash(c->qid.path); i; i = i->hash) {- if(c->qid.path == i->qid.path) {- lock(i);
- if(eqchantdqid(c, i->type, i->dev, i->qid, 0) && c->qid.type == i->qid.type)
- goto found;
- unlock(i);
- }
+ for(i = ihash(c->qid.path); i != nil; i = i->hash){+ if(eqchantdqid(c, i->type, i->dev, i->qid, 0))
+ goto found;
}
/* dump pages of inactive images to free image structures */
- while((i = imagealloc.free) == nil) {+ if((i = imagealloc.free) == nil) {unlock(&imagealloc);
if(imagereclaim(0) == 0 && imagealloc.free == nil){freebroken(); /* can use the memory */
resrcwait("no image after reclaim");}
- lock(&imagealloc);
+ goto retry;
}
-
imagealloc.free = i->next;
- lock(i);
i->type = c->type;
i->dev = c->dev;
i->qid = c->qid;
@@ -282,28 +269,67 @@
l = &ihash(c->qid.path);
i->hash = *l;
*l = i;
-
found:
+ incref(i);
unlock(&imagealloc);
+
+ lock(i);
if(i->c == nil){i->c = c;
incref(c);
}
- if(i->s == nil) {- incref(i);
- if(waserror()) {- putimage(i);
- nexterror();
+ return i;
+}
+
+/* putimage(): called with image locked and unlocks */
+void
+putimage(Image *i)
+{+ Image *f, **l;
+ Chan *c;
+ long r;
+
+ r = decref(i);
+ if(i->notext){+ unlock(i);
+ return;
+ }
+
+ /*
+ * all remaining references to this image are from the
+ * page cache, so close the chan.
+ */
+ if(r == i->pgref){+ c = i->c;
+ i->c = nil;
+ } else
+ c = nil;
+
+ if(r == 0){+ assert(i->pgref == 0);
+ assert(i->c == nil);
+ assert(i->s == nil);
+
+ lock(&imagealloc);
+ if(i->ref == 0){+ l = &ihash(i->qid.path);
+ for(f = *l; f != nil; f = f->hash) {+ if(f == i) {+ *l = i->hash;
+ break;
+ }
+ l = &f->hash;
+ }
+ i->next = imagealloc.free;
+ imagealloc.free = i;
}
- i->s = newseg(type, base, len);
- i->s->image = i;
- poperror();
+ unlock(&imagealloc);
}
- else
- incref(i->s);
+ unlock(i);
- return i;
+ if(c != nil)
+ ccloseq(c); /* does not block */
}
ulong
@@ -345,51 +371,6 @@
qunlock(&imagealloc.ireclaim);
return np;
-}
-
-/* putimage(): called with image locked and unlocks */
-void
-putimage(Image *i)
-{- Image *f, **l;
- Chan *c;
- long r;
-
- r = decref(i);
- if(i->notext){- unlock(i);
- return;
- }
-
- c = nil;
- if(r == i->pgref){- /*
- * all remaining references to this image are from the
- * page cache, so close the chan.
- */
- c = i->c;
- i->c = nil;
- }
- if(r == 0){- l = &ihash(i->qid.path);
- mkqid(&i->qid, ~0, ~0, QTFILE);
- unlock(i);
-
- lock(&imagealloc);
- for(f = *l; f != nil; f = f->hash) {- if(f == i) {- *l = i->hash;
- break;
- }
- l = &f->hash;
- }
- i->next = imagealloc.free;
- imagealloc.free = i;
- unlock(&imagealloc);
- } else
- unlock(i);
- if(c != nil)
- ccloseq(c); /* does not block */
}
uintptr
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -295,7 +295,7 @@
int i, n, indir;
ulong magic, ssize, nargs, nbytes;
uintptr t, d, b, entry, text, data, bss, bssend, tstk, align;
- Segment *s, *ts;
+ Segment *s;
Image *img;
Tos *tos;
Chan *tc;
@@ -538,23 +538,36 @@
/* Text. Shared. Attaches to cache image if possible */
/* attachimage returns a locked cache image */
- img = attachimage(SG_TEXT | SG_RONLY, tc, UTZERO, (t-UTZERO)>>PGSHIFT);
- ts = img->s;
- up->seg[TSEG] = ts;
- ts->flushme = 1;
- ts->fstart = 0;
- ts->flen = text;
- unlock(img);
+ img = attachimage(tc);
+ if((s = img->s) != nil && s->flen == text){+ assert(s->image == img);
+ incref(s);
+ putimage(img);
+ } else {+ if(waserror()){+ putimage(img);
+ nexterror();
+ }
+ s = newseg(SG_TEXT | SG_RONLY, UTZERO, (t-UTZERO)>>PGSHIFT);
+ s->flushme = 1;
+ s->image = img;
+ s->fstart = 0;
+ s->flen = text;
+ img->s = s;
+ unlock(img);
+ poperror();
+ }
+ up->seg[TSEG] = s;
/* Data. Shared. */
s = newseg(SG_DATA, t, (d-t)>>PGSHIFT);
- up->seg[DSEG] = s;
/* Attached by hand */
incref(img);
s->image = img;
- s->fstart = ts->fstart+ts->flen;
+ s->fstart = text;
s->flen = data;
+ up->seg[DSEG] = s;
/* BSS. Zero fill on demand */
up->seg[BSEG] = newseg(SG_BSS, d, (b-d)>>PGSHIFT);
@@ -580,6 +593,11 @@
}
poperror(); /* tc */
+ if(tc == img->c){+ /* avoid double caching */
+ tc->flag &= ~CCACHE;
+ cclunk(tc);
+ }
cclose(tc);
poperror(); /* file0 */
free(file0);
--
⑨