Re: [9fans] syscall silently kill processes
On Tue, 28 Jun 2022, andrey100100...@gmail.com wrote: Thanks for the patch. It's just to play with it, note that onnote should be just passed once. I'll post another patch if things work ok. adr -- 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/Tfa6823048ad90a21-M348ddb20db1a1bf17ce2b189 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] syscall silently kill processes
В Вт, 28/06/2022 в 15:28 +, adr пишет:
> Andrey, if you want to use different note handlers per process (with
> a big
> number of processes) using libthread, this may be helpful.
>
> The idea is this:
>
> An array of handlers for all processes which can be changed by all
> processes.
> When a note is received by a process, this array takes priority.
>
> An array of pointers to structures of the type
>
> struct Onnote
> {
> int pid;
> int (*fn[NFN])(void*, char*);
> };
>
> initially of size PPCHUNK (I set it to 100, experiment with that),
> but it can grow if necessary (but not shrink, I think this would
> be overkilling).
>
> These structures are allocated the first time a process record a
> handler and freed when the process exits (or by calling
> threadcancelnotes(), note that this function can free other
> processes'
> function handlers, maybe should be better to make some restrictions)
>
> The use of "in" in threadnotify(int (*f)(void*, char*), int in) is:
>
> in > 0 : set the handler for the calling process.
> in == 0 : clear the handler for the calling process.
> in == -1 : clear the handler for all processes (except those who has
> registered it already for themselves).
> in < -1 : set the handler for all processes.
>
> There is no use of threadnotify with "in < 0" in /sys/src, so nothing
> is broken.
>
> As you are using 9front and they are serving their sources with
> 9p, here is a diff to their sources. I haven't compiled it in
> 9front, though. Note that if you want to compile the system with
> this changes, you have to eliminate the copy of note.c at
> /sys/src/cmd/execnet (it seems that note.c was added afterwards as
> I thought).
>
> I haven't test it too much, this has been more like a time-destroyer
> pastime.
>
> adr
> --- /tmp/main.c
> +++ /sys/src/libthread/main.c
> @@ -28,6 +28,10 @@
> _qlockinit(_threadrendezvous);
> _sysfatal = _threadsysfatal;
> __assert = _threadassert;
> + onnote = mallocz(PPCHUNK*sizeof(uintptr), 1);
> + if(!onnote)
> + sysfatal("Malloc of size %d failed: %r",
> PPCHUNK*sizeof(uintptr));
> + onnotesize = PPCHUNK;
> notify(_threadnote);
> if(mainstacksize == 0)
> mainstacksize = 8*1024;
> --- /tmp/note.c
> +++ /sys/src/libthread/note.c
> @@ -5,9 +5,9 @@
>
> int _threadnopasser;
>
> -#define NFN 33
> #define ERRLEN 48
> typedef struct Note Note;
> +
> struct Note
> {
> Lock inuse;
> @@ -17,62 +17,155 @@
>
> static Note notes[128];
> static Note *enotes = notes+nelem(notes);
> -static int (*onnote[NFN])(void*, char*);
> -static int onnotepid[NFN];
> +Onnote **onnote;
> +int onnotesize;
> +static int (*onnoteall[NFN])(void*, char*);
> static Lock onnotelock;
>
> int
> threadnotify(int (*f)(void*, char*), int in)
> {
> - int i, topid;
> - int (*from)(void*, char*), (*to)(void*, char*);
> + int i, j;
>
> - if(in){
> - from = nil;
> - to = f;
> - topid = _threadgetproc()->pid;
> - }else{
> - from = f;
> - to = nil;
> - topid = 0;
> - }
> lock();
> - for(i=0; i - if(onnote[i]==from){
> - onnote[i] = to;
> - onnotepid[i] = topid;
> +
> + /* add note for all processes */
> + if(in < -1){
> + for(i=0; i + if(onnoteall[i] == f){
> + unlock();
> + return 1;
> + }
> + for(i=0; i + if(onnoteall[i] == nil){
> + onnoteall[i] = f;
> + break;
> + }
> + unlock();
> + return i + }
> +
> + /* remove note for all processes */
> + if(in == -1){
> + for(i=0; i + if(onnoteall[i] == f){
> + onnoteall[i] = nil;
> + break;
> + }
> + unlock();
> + return i + }
> +
> + /* remove note for current process */
> + if(!in){
> + for(i=0; i + if(onnote[i]!=nil && onnote[i]-
> >pid==_threadgetproc()->pid){
> + for(j=0; j + if(onnote[i]->fn[j] == f){
> + onnote[i]->fn[j] = 0;
> + break;
> + }
> + }
> + unlock();
> + return j + }
> + }
> + unlock();
> +
Re: [9fans] syscall silently kill processes
On Tue, 28 Jun 2022, adr wrote:
This just evade going through the arrays twice. For the current
value of NFN it doesn't make too much a difference, note that this
structures are locked. It just was hurting my eyes.
Sorry for the noise, bad patch.
--- /tmp/main.c
+++ /sys/src/libthread/main.c
@@ -28,6 +28,10 @@
_qlockinit(_threadrendezvous);
_sysfatal = _threadsysfatal;
__assert = _threadassert;
+ onnote = mallocz(PPCHUNK*sizeof(uintptr), 1);
+ if(!onnote)
+ sysfatal("Malloc of size %d failed: %r",
PPCHUNK*sizeof(uintptr));
+ onnotesize = PPCHUNK;
notify(_threadnote);
if(mainstacksize == 0)
mainstacksize = 8*1024;
--- /tmp/note.c
+++ /sys/src/libthread/note.c
@@ -5,7 +5,6 @@
int _threadnopasser;
-#defineNFN 33
#define ERRLEN 48
typedef struct Note Note;
struct Note
@@ -17,62 +16,161 @@
static Note notes[128];
static Note *enotes = notes+nelem(notes);
-static int (*onnote[NFN])(void*, char*);
-static int onnotepid[NFN];
+Onnote **onnote;
+int onnotesize;
+static int (*onnoteall[NFN])(void*, char*);
static Lock onnotelock;
int
threadnotify(int (*f)(void*, char*), int in)
{
- int i, topid;
- int (*from)(void*, char*), (*to)(void*, char*);
+ int i, j, n;
- if(in){
- from = nil;
- to = f;
- topid = _threadgetproc()->pid;
- }else{
- from = f;
- to = nil;
- topid = 0;
- }
lock();
- for(i=0; i -1)
+ onnoteall[n] = f;
+ unlock();
+ return n>-1;
+ }
+
+ /* remove note for all processes */
+ if(in == -1){
+ for(i=0; ipid==_threadgetproc()->pid){
+ for(j=0; jfn[j] == f){
+ onnote[i]->fn[j] = 0;
+ break;
+ }
+ }
+ unlock();
+ return jpid==_threadgetproc()->pid)
+ break;
+
+ /* process has already a slot */
+ if(i < onnotesize){
+ n = -1;
+ for(j=0; jfn[j] == f){
+ unlock();
+ return 1;
+ }
+ if(onnote[i]->fn[j]==nil && n==-1)
+ n = j;
+ }
+ if(n > -1)
+ onnote[i]->fn[n] = f;
+ unlock();
+ return n>-1;
+
+ }
+
+ for(i=0; ipid = _threadgetproc()->pid;
+ onnote[i]->fn[0] = f;
+ unlock();
+ return 1;
+}
+
+void
+threadcancelnotes(int pid)
+{
+ int i;
+
+ lock();
+ for(i=0; ipid==pid){
+ free(onnote[i]);
+ onnote[i] = nil;
break;
}
unlock();
- return ipending)
return;
p->pending = 0;
+ all = j = 0;
for(n=notes; nproc == p){
- strcpy(s, n->s);
- n->proc = nil;
- unlock(>inuse);
-
- for(i=0; ipid || (fn =
onnote[i])==nil)
- continue;
- if((*fn)(v, s))
- break;
+ for(i=0; is)){
+ all = 1;
+ break;
+ }
+ if(!all){
+ for(i=0; ipid==p->pid){
+ for(j=0; jfn[j])
+ if((*f)(v,
n->s))
+ break;
+ break;
+ }
}
- if(i==NFN){
- _threaddebug(DBGNOTE, "Unhandled note %s, proc
%p", n->s, p);
+ if(!all && (i==onnotesize || j==NFN)){
+ _threaddebug(DBGNOTE, "Unhandled note %s, proc
%p\n", n->s, p);
if(v != nil)
noted(NDFLT);
else if(strncmp(n->s, "sys:", 4)==0)
@@ -79,6 +177,8 @@
abort();
threadexitsall(n->s);
}
+ n->proc = nil;
+ unlock(>inuse);
}
}
}
@@ -94,7 +194,7 @@
noted(NDFLT);
if(_threadexitsallstatus){
-
Re: [9fans] syscall silently kill processes
On Tue, 28 Jun 2022, adr wrote:
Andrey, if you want to use different note handlers per process (with a big
number of processes) using libthread, this may be helpful.
The idea is this:
An array of handlers for all processes which can be changed by all processes.
When a note is received by a process, this array takes priority.
An array of pointers to structures of the type
struct Onnote
{
int pid;
int (*fn[NFN])(void*, char*);
};
initially of size PPCHUNK (I set it to 100, experiment with that),
but it can grow if necessary (but not shrink, I think this would
be overkilling).
These structures are allocated the first time a process record a
handler and freed when the process exits (or by calling
threadcancelnotes(), note that this function can free other processes'
function handlers, maybe should be better to make some restrictions)
The use of "in" in threadnotify(int (*f)(void*, char*), int in) is:
in > 0 : set the handler for the calling process.
in == 0 : clear the handler for the calling process.
in == -1 : clear the handler for all processes (except those who has
registered it already for themselves).
in < -1 : set the handler for all processes.
There is no use of threadnotify with "in < 0" in /sys/src, so nothing is
broken.
As you are using 9front and they are serving their sources with
9p, here is a diff to their sources. I haven't compiled it in
9front, though. Note that if you want to compile the system with
this changes, you have to eliminate the copy of note.c at
/sys/src/cmd/execnet (it seems that note.c was added afterwards as
I thought).
I haven't test it too much, this has been more like a time-destroyer
pastime.
This just evade going through the arrays twice. For the current
value of NFN it doesn't make too much a difference, note that this
structures are locked. It just was hurting my eyes.
--- /tmp/main.c
+++ /sys/src/libthread/main.c
@@ -28,6 +28,10 @@
_qlockinit(_threadrendezvous);
_sysfatal = _threadsysfatal;
__assert = _threadassert;
+ onnote = mallocz(PPCHUNK*sizeof(uintptr), 1);
+ if(!onnote)
+ sysfatal("Malloc of size %d failed: %r",
PPCHUNK*sizeof(uintptr));
+ onnotesize = PPCHUNK;
notify(_threadnote);
if(mainstacksize == 0)
mainstacksize = 8*1024;
--- /tmp/note.c
+++ /sys/src/libthread/note.c
@@ -5,7 +5,6 @@
int _threadnopasser;
-#defineNFN 33
#define ERRLEN 48
typedef struct Note Note;
struct Note
@@ -17,62 +16,161 @@
static Note notes[128];
static Note *enotes = notes+nelem(notes);
-static int (*onnote[NFN])(void*, char*);
-static int onnotepid[NFN];
+Onnote **onnote;
+int onnotesize;
+static int (*onnoteall[NFN])(void*, char*);
static Lock onnotelock;
int
threadnotify(int (*f)(void*, char*), int in)
{
- int i, topid;
- int (*from)(void*, char*), (*to)(void*, char*);
+ int i, j, n;
- if(in){
- from = nil;
- to = f;
- topid = _threadgetproc()->pid;
- }else{
- from = f;
- to = nil;
- topid = 0;
- }
lock();
- for(i=0; i -1)
+ onnoteall[n] = f;
+ unlock();
+ return n>-1;
+ }
+
+ /* remove note for all processes */
+ if(in == -1){
+ for(i=0; ipid==_threadgetproc()->pid){
+ for(j=0; jfn[j] == f){
+ onnote[i]->fn[j] = 0;
+ break;
+ }
+ }
+ unlock();
+ return jpid==_threadgetproc()->pid)
+ break;
+
+ /* process has already a slot */
+ if(i < onnotesize){
+ n = -1;
+ for(j=0; jfn[j] == f){
+ unlock();
+ return 1;
+ }
+ if(onnote[i]->fn[j] == nil)
+ n = j;
+ }
+ if(n > -1)
+ onnote[i]->fn[n] = f;
+ unlock();
+ return n>-1;
+
+ }
+
+ for(i=0; ipid = _threadgetproc()->pid;
+ onnote[i]->fn[0] = f;
+ unlock();
+ return 1;
+}
+
+void
+threadcancelnotes(int pid)
+{
+ int i;
+
+ lock();
+ for(i=0; ipid==pid){
+ free(onnote[i]);
+ onnote[i] = nil;
break;
}
unlock();
- return ipending)
return;
p->pending = 0;
+ all = j = 0;
for(n=notes; nproc == p){
- strcpy(s, n->s);
- n->proc = nil;
- unlock(>inuse);
-
- for(i=0;
Re: [9fans] syscall silently kill processes
Quoth adr : > Andrey, if you want to use different note handlers per process (with a big > number of processes) using libthread, this may be helpful. > > The idea is this: > > An array of handlers for all processes which can be changed by all processes. > When a note is received by a process, this array takes priority. > > An array of pointers to structures of the type take a look at privalloc; I suspect a number of libthread data structures could benefit from being thread-local. -- 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/Tfa6823048ad90a21-Mbf51d6063d50b8c495966c35 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Tue, Jun 28, 2022 at 11:38 AM adr wrote: > On Tue, 28 Jun 2022, Dan Cross wrote: > > You mean by `newthread` and `chancreate`? Those are part of the > > thread library. Notice that there are no callers outside of > > /sys/src/libthread. > > What I mean is that "size" in _threadmalloc() will be set by those > functions with values directly given by the programmer, with this > limit not documented. Like I said earlier, plan9 had the luxury of being a research system. It has brilliant ideas, but mostly trapped inside of research-quality code. If you look just below the surface, there are arbitrary limits and edge cases all over the system. It's honestly surprising that it works as well as it does. That this particular limit is not documented isn't terribly surprising. Most of these limits are undocumented. I doubt anyone ever thought to create a 100MB stack or channel when that code was written. > I wouldn't call a function wich is part of an api internal. An > internal function, for me, is a function inaccesible for the > programmer, like _threadmalloc itself. > > By the way, you mean threadcreate, don't you? No, I meant the direct calls to `_threadmalloc`. But sure, we can say `theadcreate` since that just expands to a call to `newthead` and `newthread` is static. We may as well throw `proccreate` into the mix too as it also indirectly calls `_threadmalloc` via `_newproc`. For that matter, libthead's `main` also calls `_threadmalloc`. I'm not sure if that changes the point, though. - Dan C. -- 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-M71941c1e8258c2d786a38352 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Tue, 28 Jun 2022, Dan Cross wrote: You mean by `newthread` and `chancreate`? Those are part of the thread library. Notice that there are no callers outside of /sys/src/libthread. What I mean is that "size" in _threadmalloc() will be set by those functions with values directly given by the programmer, with this limit not documented. I wouldn't call a function wich is part of an api internal. An internal function, for me, is a function inaccesible for the programmer, like _threadmalloc itself. By the way, you mean threadcreate, don't you? Regars, adr. -- 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-M0c09e2ed1ced2383c89f1fe9 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] syscall silently kill processes
Andrey, if you want to use different note handlers per process (with a big
number of processes) using libthread, this may be helpful.
The idea is this:
An array of handlers for all processes which can be changed by all processes.
When a note is received by a process, this array takes priority.
An array of pointers to structures of the type
struct Onnote
{
int pid;
int (*fn[NFN])(void*, char*);
};
initially of size PPCHUNK (I set it to 100, experiment with that),
but it can grow if necessary (but not shrink, I think this would
be overkilling).
These structures are allocated the first time a process record a
handler and freed when the process exits (or by calling
threadcancelnotes(), note that this function can free other processes'
function handlers, maybe should be better to make some restrictions)
The use of "in" in threadnotify(int (*f)(void*, char*), int in) is:
in > 0 : set the handler for the calling process.
in == 0 : clear the handler for the calling process.
in == -1 : clear the handler for all processes (except those who has
registered it already for themselves).
in < -1 : set the handler for all processes.
There is no use of threadnotify with "in < 0" in /sys/src, so nothing is broken.
As you are using 9front and they are serving their sources with
9p, here is a diff to their sources. I haven't compiled it in
9front, though. Note that if you want to compile the system with
this changes, you have to eliminate the copy of note.c at
/sys/src/cmd/execnet (it seems that note.c was added afterwards as
I thought).
I haven't test it too much, this has been more like a time-destroyer
pastime.
adr
--- /tmp/main.c
+++ /sys/src/libthread/main.c
@@ -28,6 +28,10 @@
_qlockinit(_threadrendezvous);
_sysfatal = _threadsysfatal;
__assert = _threadassert;
+ onnote = mallocz(PPCHUNK*sizeof(uintptr), 1);
+ if(!onnote)
+ sysfatal("Malloc of size %d failed: %r",
PPCHUNK*sizeof(uintptr));
+ onnotesize = PPCHUNK;
notify(_threadnote);
if(mainstacksize == 0)
mainstacksize = 8*1024;
--- /tmp/note.c
+++ /sys/src/libthread/note.c
@@ -5,9 +5,9 @@
int _threadnopasser;
-#defineNFN 33
#define ERRLEN 48
typedef struct Note Note;
+
struct Note
{
Lockinuse;
@@ -17,62 +17,155 @@
static Note notes[128];
static Note *enotes = notes+nelem(notes);
-static int (*onnote[NFN])(void*, char*);
-static int onnotepid[NFN];
+Onnote **onnote;
+int onnotesize;
+static int (*onnoteall[NFN])(void*, char*);
static Lock onnotelock;
int
threadnotify(int (*f)(void*, char*), int in)
{
- int i, topid;
- int (*from)(void*, char*), (*to)(void*, char*);
+ int i, j;
- if(in){
- from = nil;
- to = f;
- topid = _threadgetproc()->pid;
- }else{
- from = f;
- to = nil;
- topid = 0;
- }
lock();
- for(i=0; ipid==_threadgetproc()->pid){
+ for(j=0; jfn[j] == f){
+ onnote[i]->fn[j] = 0;
+ break;
+ }
+ }
+ unlock();
+ return jpid==_threadgetproc()->pid)
+ break;
+
+ /* process has already a slot */
+ if(i < onnotesize){
+ for(j=0; jfn[j] == nil){
+ onnote[i]->fn[j] = f;
+ break;
+ }
+ }
+ unlock();
+ return j+
+ }
+
+ for(i=0; ipid = _threadgetproc()->pid;
+ onnote[i]->fn[0] = f;
+ unlock();
+ return 1;
+}
+
+void
+threadcancelnotes(int pid)
+{
+ int i;
+
+ lock();
+ for(i=0; ipid==pid){
+ free(onnote[i]);
+ onnote[i] = nil;
break;
}
unlock();
- return ipending)
return;
p->pending = 0;
+ all = j = 0;
for(n=notes; nproc == p){
- strcpy(s, n->s);
- n->proc = nil;
- unlock(>inuse);
-
- for(i=0; ipid || (fn =
onnote[i])==nil)
- continue;
- if((*fn)(v, s))
- break;
+ for(i=0; is)){
+ all = 1;
+ break;
+ }
+ if(!all){
+ for(i=0; ipid==p->pid){
+ for(j=0; jfn[j])
+
Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Tue, Jun 28, 2022 at 10:22 AM adr wrote: > On Tue, 28 Jun 2022, Dan Cross wrote: > > [snip] > > Given the name of the function (`_threadmalloc`), I'd guess that this isn't > > intended for general use, but rather, for the internal consumption of the > > thread library, where indeed such a large allocation would likely be an > > error (bear in mind this code was developed on 32-bit machines with RAMs > > measured in Megabytes, not Gigabytes). > > No, it's used also when creating a channel and setting a thread's > stack size. You mean by `newthread` and `chancreate`? Those are part of the thread library. Notice that there are no callers outside of /sys/src/libthread. - Dan C. -- 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-M5755b12d8fc8f5fc46f51c97 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Tue, 28 Jun 2022, Dan Cross wrote:
[...]
void*
_threadmalloc(long size, int z)
{
void *m;
m = malloc(size);
if (m == nil)
sysfatal("Malloc of size %ld failed: %r", size);
setmalloctag(m, getcallerpc());
totalmalloc += size;
if (size > 1) {
fprint(2, "Malloc of size %ld, total %ld\n", size,
totalmalloc);
abort();
}
if (z)
memset(m, 0, size);
return m;
}
[...]
Shouldn't the if statement test the size of totalmalloc before the
abort? That size, 100M for internal allocations? It has to be
totalmalloc, doesn't it? If not this if statement should be after
testing the success of the malloc. Am I missing something?
Note that the `if` statement doesn't test the size of *totalmalloc*, but
just `size`. `totalsize` is only incidentally printed in the output if `size`
exceeds 100 MB: that is, if a single allocation is larger than 100MB.
I know, what called my attention is that size wasn't tested before
calling malloc, so the first thought I had was that the code had
a typo and it should be testing totalmalloc instead.
I mean, I think using libthread more like using small channels and thread's
stack sizes, but:
Why put a limit here to size when totalmalloc could continue to grow until
malloc fails?
If we want a limit, why don't we take into account the system resources?
If we want a constant limit, why don't we put it on threadimpl.h, explain why
this value in a comment
and document it in thread(2)?
Why bother to call malloc before testing if size exeeds that limit???
Given the name of the function (`_threadmalloc`), I'd guess that this isn't
intended for general use, but rather, for the internal consumption of the
thread library, where indeed such a large allocation would likely be an
error (bear in mind this code was developed on 32-bit machines with RAMs
measured in Megabytes, not Gigabytes).
No, it's used also when creating a channel and setting a thread's
stack size.
adr
--
9fans: 9fans
Permalink:
https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-Mcbc484781129f693c26365ef
Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Tue, Jun 28, 2022 at 9:01 AM adr wrote:
> On Sun, 26 Jun 2022, adr wrote:
> > [snip]
> > /sys/src/libthread/lib.c
> >
> > [...]
> > void*
> > _threadmalloc(long size, int z)
> > {
> > void *m;
> >
> > m = malloc(size);
> > if (m == nil)
> > sysfatal("Malloc of size %ld failed: %r", size);
> > setmalloctag(m, getcallerpc());
> > totalmalloc += size;
> > if (size > 1) {
> > fprint(2, "Malloc of size %ld, total %ld\n", size,
> > totalmalloc);
> > abort();
> > }
> > if (z)
> > memset(m, 0, size);
> > return m;
> > }
> > [...]
> >
> > Shouldn't the if statement test the size of totalmalloc before the
> > abort? That size, 100M for internal allocations? It has to be
> > totalmalloc, doesn't it? If not this if statement should be after
> > testing the success of the malloc. Am I missing something?
Note that the `if` statement doesn't test the size of *totalmalloc*, but
just `size`. `totalsize` is only incidentally printed in the output if `size`
exceeds 100 MB: that is, if a single allocation is larger than 100MB.
> I mean, I think using libthread more like using small channels and thread's
> stack sizes, but:
>
> Why put a limit here to size when totalmalloc could continue to grow until
> malloc fails?
>
> If we want a limit, why don't we take into account the system resources?
>
> If we want a constant limit, why don't we put it on threadimpl.h, explain why
> this value in a comment
> and document it in thread(2)?
>
> Why bother to call malloc before testing if size exeeds that limit???
Given the name of the function (`_threadmalloc`), I'd guess that this isn't
intended for general use, but rather, for the internal consumption of the
thread library, where indeed such a large allocation would likely be an
error (bear in mind this code was developed on 32-bit machines with RAMs
measured in Megabytes, not Gigabytes).
As for why the abort is after the allocation? The sad answer, as with so many
things in plan 9, is that there probably isn't a particularly good
reason. A possible
answer may be that this leaves the process in a state where the allocations can
be inspected in a debugger after the abort, complete with a proper
allocation tag.
An equally plausible answer is that it's just the way the original author typed
it in.
Please understand that Plan 9 spent most of its life as a research system,
and the code and its quality reflects that.
- Dan C.
--
9fans: 9fans
Permalink:
https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-M6c48d66d6cc15f19fd28066c
Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
[9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?
On Sun, 26 Jun 2022, adr wrote:
Date: Sun, 26 Jun 2022 09:50:19 + (UTC)
From: adr
To: 9fans@9fans.net
Subject: _threadmalloc() size>1; shouldn't be totalmalloc?
/sys/src/libthread/lib.c
[...]
void*
_threadmalloc(long size, int z)
{
void *m;
m = malloc(size);
if (m == nil)
sysfatal("Malloc of size %ld failed: %r", size);
setmalloctag(m, getcallerpc());
totalmalloc += size;
if (size > 1) {
fprint(2, "Malloc of size %ld, total %ld\n", size,
totalmalloc);
abort();
}
if (z)
memset(m, 0, size);
return m;
}
[...]
Shouldn't the if statement test the size of totalmalloc before the
abort? That size, 100M for internal allocations? It has to be
totalmalloc, doesn't it? If not this if statement should be after
testing the success of the malloc. Am I missing something?
adr.
I mean, I think using libthread more like using small channels and thread's
stack sizes, but:
Why put a limit here to size when totalmalloc could continue to grow until
malloc fails?
If we want a limit, why don't we take into account the system resources?
If we want a constant limit, why don't we put it on threadimpl.h, explain why
this value in a comment
and document it in thread(2)?
Why bother to call malloc before testing if size exeeds that limit???
adr
--
9fans: 9fans
Permalink:
https://9fans.topicbox.com/groups/9fans/Te1be8dc72738258d-Mf7cdc7010affd696392c0b47
Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
