Re: [9fans] Re: _threadmalloc() size>100000000; shouldn't be totalmalloc?

[Dan Cross <crossd@gmail.com>]

On Tue, Jun 28, 2022 at 9:01 AM adr <adr@sdf.org> 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(&size));
> >       totalmalloc += size;
> >       if (size > 100000000) {
> >               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