From owner-cvs-all Mon Jan 29 19:44:35 2001 Delivered-To: cvs-all@freebsd.org Received: from VL-MS-MR003.sc1.videotron.ca (relais.videotron.ca [24.201.245.36]) by hub.freebsd.org (Postfix) with ESMTP id 610A737B6A6; Mon, 29 Jan 2001 19:44:02 -0800 (PST) Received: from jehovah ([24.201.144.31]) by VL-MS-MR003.sc1.videotron.ca (Netscape Messaging Server 4.15) with SMTP id G7YIDA01.JW6; Mon, 29 Jan 2001 22:43:58 -0500 Message-ID: <003301c08a6f$2022d310$1f90c918@jehovah> From: "Bosko Milekic" To: "Alfred Perlstein" , "Peter Wemm" Cc: "Boris Popov" , , References: <200101300240.f0U2ei459361@mobile.wemm.org> Subject: Re: cvs commit: src/sys/kern kern_malloc.c src/sys/sys malloc.h Date: Mon, 29 Jan 2001 22:45:44 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 5.00.2919.6700 X-MimeOLE: Produced By Microsoft MimeOLE V5.00.2919.6700 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Peter Wemm wrote: [...] > > > Why is this change necessary? Rather, how is this change correct? I'd > > > rather not introduce this sort of thing into the actual interface, > > > unless it's realistically necessary, as I can see how this may > > > encourage some people writing drivers (or an equivalent) to decide > > > that they ought to panic the machine if they can't allocate. I'd > > > rather see this dealt with, where absolutely necessary, by calling > > > malloc() with M_NOWAIT and checking the return value and then calling > > > panic explicitly if it is NULL. > > > > I agree with Bosko on this one, it's pretty wrong, we had many > > problems in 2.2.x and early 3.x systems because of just giving up > > on resource shortages (mbufs), we should be able to eventually fix > > any places where a NULL return from malloc can't be handled, not > > encourage giving up. > > Yes, in an ideal world, we should always have a recovery path. But we do > not in all cases. Yes, this should be discouraged. M_WAITOK | M_PANIC > will generally die only if kmem_map fills up. The system is truely in > trouble at that point anyway, and if we didn't kill it gracedully then one > of the many other places that dont check returns from malloc(foo, M_WAITOK) > will get a page fault moments later. Ack! I forgot that kmem_malloc() is broken in the sense that if the map is starved and we're M_NOWAIT, it will return 0. If we're M_WAITOK, on the other hand, it will just call panic() itself. The only exception is mb_map in which case it always returns 0, but then the mbuf code deals with this by taking care of sleeping itself. malloc() should either be made to do similar or, better yet, kmem_malloc() should be fixed for malloc() usage. The difference between allocating wired-down pages from kmem_map and mb_map is that, for efficiency purposes, we never free pages back to mb_map; so perhaps it would make more sense to deal with the failure directly in the VM code for the kmem_map case. What should have been done with M_PANIC is, in my opinion, not introduce the flag to malloc(). It is a temporary solution anyway and as far as temporary solutions go, an appropriate modification would have been to make kmem_malloc() call panic() itself when it can't find address space in the map regardless of whether we called with M_WAITOK or M_NOWAIT - and a comment should probably be placed there to remind us that we ought to fix this. > IMHO, it is *far* better to get a panic than a page fault. Getting neither > is better still, but we do not live in that ideal world yet. Uhm, if we're page faulting, it's likely because we have a bug in the first place. malloc() is never supposed to return if we're calling it with M_WAITOK. In fact, if you call malloc() with M_WAITOK and we can't find the address space in the map (the map is starved), kmem_malloc() will panic itself. Therefore, the only case where one must check the return value of malloc() is when calling it with M_NOWAIT. At least this is how it's documented, and it appears that the code matches the documentation, in this case. So if we're calling malloc() with M_NOWAIT and not checking the return value, that should be immediately classified as a "bug," and we should not be introducing a flag to malloc() to fix it. > Cheers, > -Peter > -- > Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au > "All of this is for nothing if we don't go to the stars" - JMS/B5 Regards, Bosko. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message