From owner-freebsd-current@FreeBSD.ORG Fri Jul 2 18:46:14 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5DA6116A4CE; Fri, 2 Jul 2004 18:46:14 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3F49B43D31; Fri, 2 Jul 2004 18:46:14 +0000 (GMT) (envelope-from bmilekic@FreeBSD.org) Received: from freefall.freebsd.org (bmilekic@localhost [127.0.0.1]) i62IhONW011716; Fri, 2 Jul 2004 18:43:24 GMT (envelope-from bmilekic@freefall.freebsd.org) Received: (from bmilekic@localhost) by freefall.freebsd.org (8.12.11/8.12.11/Submit) id i62IhOVC011715; Fri, 2 Jul 2004 18:43:24 GMT (envelope-from bmilekic) Date: Fri, 2 Jul 2004 18:43:24 +0000 From: Bosko Milekic To: Brian Fundakowski Feldman Message-ID: <20040702184324.GA10170@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.1i cc: current@freebsd.org Subject: Re: UMA zinit/zctor function error returning X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Jul 2004 18:46:14 -0000 Brian Fundakowski Feldman wrote: > Good work, overall. I've looked at it, here are comments: - in mb_ctor_mbuf(), you need to free the mbuf if mac_init_mbuf() failed, before you return error (SERIOUS). - return (0) should be "return 0" (STYLE). - in mb_ctor_pack(), you need to free the mbuf if mac_init_mbuf() failed, before you return error (SERIOUS). - you didn't make the change for keginit and you split out the init function types into init (for zones) and keginit (for kegs). However, keg inits should be able to fail too and it should probably be the SAME type as the zone init. This is a mistake. It is especially a mistake since most zones actually use the keg init, not the zone init (by default). Only if you add a secondary zone do you get to talk about zone inits. Let me know if this isn't clear for you. - You seem to misunderstand where inits are called. Refer to my netbuf paper at www.unixdaemons.com/~bmilekic/netbuf_bmilekic.pdf (specifically the diagrams) to see where the zone init and keg inits are called; for example, I have no idea why you've now defined zero_keginit and zero_init, this makes no sense. This part of the patch (including how you now cast to uma_keginit explicitly in zone_ctor) needs to be redone. I don't think you should split up the init types (i.e., zone init and keg init have same types) and have both of them be able to fail. I know you might find this tougher since you have to deal with freeing back to the slab from the keg code on allocation failure, but that complexity is part of this change's requirement. - the uma_dbg.c changes call the ctor explicitly from the fini. I forget the exact reason this is, but now that we're expecting the ctor to return something, if you are ignoring the return value, please cast void explicitly here (STYLE). Alternately, make both keg and zone inits able to fail and propagate the failure, if any, upwards. Regards, -Bosko