Subject: Re: [Cbe-oss-dev] [PATCH 12/16] cell: add temperature to SPU and CPU sysfs entries

From: Benjamin Herrenschmidt <[email-masked]>
Date: Thu, 26 Oct 2006 09:19:24 +1000

On Wed, 2006-10-25 at 10:00 +0200, Heiko Carstens wrote:
> On Tue, Oct 24, 2006 at 06:31:25PM +0200, [email-masked] wrote:
> 
> > + * (C) Copyright IBM Deutschland Entwicklung GmbH 2005
> 
> IBM Corp. instead of IBM DE? 2006?
> 
> > +static int __init thermal_init(void)
> > +{
> > +	init_default_values();
> > +
> > +	spu_add_sysdev_attr_group(&spu_attribute_group);
> > +	cpu_add_sysdev_attr_group(&ppe_attribute_group);
> > +
> > +	return 0;
> > +}
> 
> Same here: check for errors on spu_add_sysdev_attr_group and
> cpu_add_sysdev_attr_group.
> 
> > +static void __exit thermal_exit(void)
> > +{
> > +	spu_remove_sysdev_attr_group(&spu_attribute_group);
> > +	cpu_remove_sysdev_attr_group(&ppe_attribute_group);
> 
> Will crash if cpu_add_sysdev_attr_group failed...

Which is a total PITA. If this is the case, then we should modify the
add calls to at least initialize enough fields before they can fail for
the remove calls not to crash. You don't want to keep track precisely of
what file was added and what not and test all of that in your exit code
path, it's just insane.

Ben.