UMA zinit/zctor function error returning
Bosko Milekic
bmilekic at FreeBSD.org
Fri Jul 2 11:46:14 PDT 2004
Brian Fundakowski Feldman wrote:
> <URL:http://green.homeunix.org/~green/uma_error_checking.patch>
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
More information about the freebsd-current
mailing list