From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 1927 invoked from network); 12 Nov 2022 10:21:37 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 12 Nov 2022 10:21:37 -0000 Received: from cc-smtpout2.netcologne.de ([89.1.8.212]) by 9front; Sat Nov 12 05:20:18 -0500 2022 Received: from cc-app2.netcologne.de (cc-app2.netcologne.de [89.1.9.191]) by cc-smtpout2.netcologne.de (Postfix) with ESMTP id 587F0124C2 for <9front@9front.org>; Sat, 12 Nov 2022 11:20:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=netcologne.de; s=nc1116a; t=1668248413; bh=L+dzyoNmOQMShUAElB5de92XpNcFpUMsBs/E9fIUBYI=; h=Date:From:To:Message-ID:In-Reply-To:References:Subject:From; b=UoaFSTPGirv7L0MEWNijKZLo4bt1NXVSeKJ8AxHm40+B6fdQmTo1w+9eXAY1V0WDt DE4CRWgPVO1TwsuR5JADSnIB3/A4XaeC4o+qhnB5aZQwCHxGoCOzh+WxVkT+/zjKpv nTAbhrVeVn7QgWCibDDqeTCDpHLGzkHvuxjx5cuz5aKNpRMayU+RCpmy2akHVTpkwa t7VK7qoa/O1Z29XreCU8OYbSKQETSH5M95dHsN7vjjvRDGCqPIWXjskosZVmyUL5vy HUNShTDEm9/bYmAkVP6TENjryb73m2ykqK8hfc8T/7GsGAJjP22nXq53rY6fGgbIb5 wONErH0ndr58A== Received: from cc-app2.netcologne.de (localhost [127.0.0.1]) by cc-app2.netcologne.de (Postfix) with ESMTPA id D965E11E02 for <9front@9front.org>; Sat, 12 Nov 2022 11:20:12 +0100 (CET) Date: Sat, 12 Nov 2022 11:20:12 +0100 (CET) From: Arne Meyer To: 9front@9front.org Message-ID: <305235037.4605417.1668248412799@comcenter.netcologne.de> In-Reply-To: <94a397b1-daa1-2ad2-137b-64dbbd150065@posixcafe.org> References: <2110680159.4595621.1668187065209@comcenter.netcologne.de> <6c58605e-7b25-1adc-3681-41df77e4ae91@posixcafe.org> <1453278421.4596864.1668189134709@comcenter.netcologne.de> <94a397b1-daa1-2ad2-137b-64dbbd150065@posixcafe.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.6-Rev16 X-Originating-IP: 89.1.210.201 X-Originating-Client: open-xchange-appsuite X-NetCologne-Spam: L X-Rspamd-Queue-Id: D965E11E02 X-Rspamd-Pre-Result: action=add header; module=force_actions; unknown reason X-Spamd-Bar: / List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: distributed ISO-certified deep-learning HTML component self-signing-aware manager Subject: Re: [9front] [patch] skelfs: remove useless code Reply-To: 9front@9front.org Precedence: bulk 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 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 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){