All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	H Peter Anvin <hpa@zytor.com>, Len Brown <lenb@kernel.org>,
	Jin Dongming <jin.dongming@np.css.fujitsu.com>,
	Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>,
	Jean Delvare <khali@linux-fr.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	lm-sensors <lm-sensors@lm-sensors.org>
Subject: Re: [lm-sensors] [PATCH] therm_throt.c: Fix error handling in
Date: Sun, 05 Sep 2010 12:32:29 +0000	[thread overview]
Message-ID: <20100905123229.GA8254@elte.hu> (raw)
In-Reply-To: <20100903213456.GA22124@fenghua-desk.sc.intel.com>


* Fenghua Yu <fenghua.yu@intel.com> wrote:

> Or other options could be:
> 
> 1. Just calling sysfs_add_file_to_group() without collecting returned error and
> return 0 at the end (driver/pci/pcie/aspm.c does like this). The drawback is
> there is no error logged if an unlikely errorr occurs. But user can see some
> files are missing in sysfs.
>
> 2. Or collect errors in err1, err2, etc for each sysfs_add_file_to_group. At
> the end, return -ENODEV(??) if any err1, err2, etc is not 0. This option makes
> code unreasonable complex to handle unlikely errors.

Well, the usual way to handle errors is to abort the operation when it 
occurs, and return the error code that sysfs_add_file_to_group() gave.

The error is not 'fatal' but missing sysfs files sure are confusing, and 
might break user-land which depends on them. So we should either 
initialize a driver fully - or not intialize it at all.

Now, a sub-case is the question whether to emit something more than the 
return code from sysfs_add_file_to_group(). If it's exceedingly rare 
(and subsequently poorly tested) then adding a WARN_ON_ONCE(ret) is OK - 
but that error code should be returned.

Am i missing any detail?

Thanks,

	Ingo

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@elte.hu>
To: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	H Peter Anvin <hpa@zytor.com>, Len Brown <lenb@kernel.org>,
	Jin Dongming <jin.dongming@np.css.fujitsu.com>,
	Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>,
	Jean Delvare <khali@linux-fr.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	lm-sensors <lm-sensors@lm-sensors.org>
Subject: Re: [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev
Date: Sun, 5 Sep 2010 14:32:29 +0200	[thread overview]
Message-ID: <20100905123229.GA8254@elte.hu> (raw)
In-Reply-To: <20100903213456.GA22124@fenghua-desk.sc.intel.com>


* Fenghua Yu <fenghua.yu@intel.com> wrote:

> Or other options could be:
> 
> 1. Just calling sysfs_add_file_to_group() without collecting returned error and
> return 0 at the end (driver/pci/pcie/aspm.c does like this). The drawback is
> there is no error logged if an unlikely errorr occurs. But user can see some
> files are missing in sysfs.
>
> 2. Or collect errors in err1, err2, etc for each sysfs_add_file_to_group. At
> the end, return -ENODEV(??) if any err1, err2, etc is not 0. This option makes
> code unreasonable complex to handle unlikely errors.

Well, the usual way to handle errors is to abort the operation when it 
occurs, and return the error code that sysfs_add_file_to_group() gave.

The error is not 'fatal' but missing sysfs files sure are confusing, and 
might break user-land which depends on them. So we should either 
initialize a driver fully - or not intialize it at all.

Now, a sub-case is the question whether to emit something more than the 
return code from sysfs_add_file_to_group(). If it's exceedingly rare 
(and subsequently poorly tested) then adding a WARN_ON_ONCE(ret) is OK - 
but that error code should be returned.

Am i missing any detail?

Thanks,

	Ingo

  reply	other threads:[~2010-09-05 12:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-03  0:45 [lm-sensors] [PATCH] therm_throt.c: Fix error handling in Fenghua Yu
2010-09-03  0:45 ` [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev Fenghua Yu
2010-09-03  8:48 ` [lm-sensors] [PATCH] therm_throt.c: Fix error handling in Jean Delvare
2010-09-03  8:48   ` [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev Jean Delvare
2010-09-03  9:35 ` [lm-sensors] [PATCH] therm_throt.c: Fix error handling in Ingo Molnar
2010-09-03  9:35   ` [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev Ingo Molnar
2010-09-03 21:34   ` [lm-sensors] [PATCH] therm_throt.c: Fix error handling Fenghua Yu
2010-09-03 21:34     ` [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev Fenghua Yu
2010-09-05 12:32     ` Ingo Molnar [this message]
2010-09-05 12:32       ` Ingo Molnar
2010-09-05 16:35       ` [lm-sensors] [PATCH] therm_throt.c: Fix error handling in Guenter Roeck
2010-09-05 16:35         ` [lm-sensors] [PATCH] therm_throt.c: Fix error handling in thermal_throttle_add_dev Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100905123229.GA8254@elte.hu \
    --to=mingo@elte.hu \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=jin.dongming@np.css.fujitsu.com \
    --cc=khali@linux-fr.org \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=seto.hidetoshi@jp.fujitsu.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.