Re: Why is device_create_file __must_check?

[Greg KH <greg@kroah.com>]

On Tue, Oct 10, 2006 at 03:14:04PM +1000, Paul Mackerras wrote:
> Andrew Morton writes:
> 
> > There are no super-strong reasons here, but if device_create_file() fails
> > then the required control files aren't there and the subsystem isn't
> > working as intended.  If it's in a module then we should fail the modprobe. 
> > If it's a bootup thing then best we can do is to panic.  Or at least log
> > the event.
> 
> In the case of the windfarm driver, the sysfs files are reporting
> things like cpu voltage, current, temperature etc. which can be
> interesting to know about, but the sysfs files are not essential to
> the operation of the driver.  So just some cheesy printk would do in
> that sort of situation, I guess.

Then add a cheesy printk into your driver then.  That's fine.

> > The most common cause of this is a programming error: we tried to create
> > the same entry twice.   We want to know about that.
> 
> In that case a WARN_ON inside device_create_file when the duplicate is
> detected would be better - less code, and only one place where the
> check needs to be done.  The WARN_ON will give us a backtrace so we
> can see where the second creation attempt happened.
> 
> > Because it can fail.  We need to take _some_ action if the setup failed - at
> > least report it so the user (and the kernel developers) know that something
> > is going wrong.
> 
> So we have to add printks in all sorts of places where the
> device_create_file has never failed before.  If you're that concerned,
> why not add a WARN_ON(error) in device_create_file() ?

Because it's kind of been proven that not even then are things fixed up
(by personal experience with people naming drivers the same thing, which
will cause an WARN_ON() in the kernel to happen, yet is ignored by the
developer.)

But I don't mind adding it there too, if that helps.  But again, even
then, don't ignore the return value, it's trying to tell you something.
If you have multiple sysfs files, just use an attribute group which will
be be unwound automatically if something goes wrong.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/