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.2 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, NICE_REPLY_A autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 8636 invoked from network); 11 Nov 2022 19:24:10 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 11 Nov 2022 19:24:10 -0000 Received: from mail.posixcafe.org ([45.76.19.58]) by 9front; Fri Nov 11 14:22:43 -0500 2022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=posixcafe.org; s=20200506; t=1668194686; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=O+ndLHmBxsAbA4aaFgpO75lX2njE5XfY1Nhc4Ptbhrc=; b=I5Yr4KMijvQ3nQIxXG4KChGD3GWy4MgvUxiDbPMWkZx8QGOKE8Sf0MklU0hlkqGjdbo+R5 qX9AqNc51gbZA9OxfZJxNgWMdGDpBawiOSzxMCQuqq4PRk3wkPjLCgQBn6Vl5YDqIJPpgW 1jogAwhQTbJXrvf/XPOSPnlivYA3eh8= Received: from [192.168.168.200] (161-97-205-25.mynextlight.net [161.97.205.25]) by mail.posixcafe.org (OpenSMTPD) with ESMTPSA id 77a51893 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for <9front@9front.org>; Fri, 11 Nov 2022 13:24:46 -0600 (CST) Message-ID: <94a397b1-daa1-2ad2-137b-64dbbd150065@posixcafe.org> Date: Fri, 11 Nov 2022 12:20:25 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Content-Language: en-US To: 9front@9front.org References: <2110680159.4595621.1668187065209@comcenter.netcologne.de> <6c58605e-7b25-1adc-3681-41df77e4ae91@posixcafe.org> <1453278421.4596864.1668189134709@comcenter.netcologne.de> From: Jacob Moody In-Reply-To: <1453278421.4596864.1668189134709@comcenter.netcologne.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: immutable optimized database table Subject: Re: [9front] [patch] skelfs: remove useless code Reply-To: 9front@9front.org Precedence: bulk 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){