From: Arne Meyer <meyer.arne83@netcologne.de>
To: 9front@9front.org
Subject: Re: [9front] [patch] skelfs: remove useless code
Date: Sat, 12 Nov 2022 11:20:12 +0100 (CET) [thread overview]
Message-ID: <305235037.4605417.1668248412799@comcenter.netcologne.de> (raw)
In-Reply-To: <94a397b1-daa1-2ad2-137b-64dbbd150065@posixcafe.org>
If we are failing to allocate, why try to keep things running? We could just sysfatal. Most emalloc functions do exactly that, and some code uses both emalloc and emalloc9p.
> Jacob Moody <moody@mail.posixcafe.org> hat am 11.11.2022 19:20 GMT geschrieben:
>
>
> On 11/11/22 10:52, Arne Meyer wrote:
> > After that patch I grepped through /sys/src/cmd/* and found a more hits. Is there any difference between handrolled emalloc implementations and emalloc9p? If not, I can go spelunking to replace emalloc where emalloc9p is available.
> >
> >> Jacob Moody <moody@mail.posixcafe.org> hat am 11.11.2022 17:37 GMT geschrieben:
> >>
> >>
> >> On 11/11/22 10:17, Arne Meyer wrote:
> >>> Hello,
> >>>
> >>> if I read the code correctly emalloc9p already checks for failed allocations and zeroes the memory, so the following code is redundant.
> >>>
> >>> Greetings,
> >>> Arne
> >>
> >> You are correct, muscle memory did not work in my favor this time.
> >>
> >> Thanks!
> >> moody
>
> The only difference I generally see is that the emalloc9p and friends will call exits("mem"), which in the context
> of lib9p (generally) just means the process for that specific pipe is killed. I believe the intention here is to allow other srv
> procs to continue existing, but if that is the case we surely are doing it half assed right now.
>
> For starters we should probably be using threadexits() when we're threaded, and we're surely leaking entries in the
> fidmaps and the Fid structs themselves. Ideally those functions would be given enough context to properly unroll
> the entire process state, if we did want to really handle this case. Of course this is only for file systems
> that do have that listen/fork loop, if you are just posting to /srv then you will only ever have the one service
> proc. But then of course for threaded systems you have to communicate that things blew up, or else your srv
> proc could just die and leave everything else deadlocked.
>
> So here's my shot in the dark for something that could be a bit better, if we had the e* functions in mem.c
> take a Request as the first argument, then perhaps provide a destroysrv callback for user code to clean things up
> associated with that pipe. With that those functions have a better idea of how to clean up the mess. But these error
> handling unrolling things are almost all some brand of awful, it may not be worth the effort to do 'better'.
>
> While reviewing the lib9p code for these thoughts, I came across a scenario of lib9p checking for this
> stuff itself, this changes that:
>
> diff a39eb30ffa2138a817bd787fdfd25eb65927e7d4 uncommitted
> --- a//sys/src/lib9p/srv.c
> +++ b//sys/src/lib9p/srv.c
> @@ -666,10 +666,6 @@
> }
> n = GBIT16(tmp)+BIT16SZ;
> statbuf = emalloc9p(n);
> - if(statbuf == nil){
> - r->error = "out of memory";
> - return;
> - }
> r->ofcall.nstat = convD2M(&r->d, statbuf, n);
> r->ofcall.stat = statbuf; /* freed in closereq */
> if(r->ofcall.nstat <= BIT16SZ){
prev parent reply other threads:[~2022-11-12 10:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-11 17:17 Arne Meyer
2022-11-11 17:37 ` Jacob Moody
2022-11-11 17:52 ` Arne Meyer
2022-11-11 19:20 ` Jacob Moody
2022-11-12 10:20 ` Arne Meyer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=305235037.4605417.1668248412799@comcenter.netcologne.de \
--to=meyer.arne83@netcologne.de \
--cc=9front@9front.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).