more sysfs and driver core patches for 2.6.22
Subsystems (8)
PATCH 1/1 more sysfs and driver core patches for 2.6.22
Here are some more driver core and sysfs patches against your 2.6.22 git tree.
They fix some bugs in the last big sysfs merge (thanks to Tejun for finding and fixing them so quickly) and some other minor stuff all over the place.
Some of these have been in the -mm tree for a while.
Please pull from:
master.kernel.org:/pub/scm/linux/kernel/git/gregkh/driver-2.6.git/
Patches will be sent as a follow-on to this message to lkml for people to see.
thanks,
greg k-h
Documentation/CodingStyle | 20 +++- Documentation/HOWTO | 3 + Documentation/driver-model/devres.txt | 2 +- Documentation/feature-removal-schedule.txt | 4 +- drivers/base/core.c | 168 +++++++++++++++++++++------- drivers/base/power/Makefile | 2 +- drivers/base/power/power.h | 5 - drivers/base/power/runtime.c | 85 -------------- drivers/base/power/sysfs.c | 66 ----------- drivers/pcmcia/ds.c | 40 ++++++- drivers/usb/core/driver.c | 7 - fs/debugfs/inode.c | 5 - fs/sysfs/dir.c | 25 +++-- fs/sysfs/file.c | 9 +- fs/sysfs/inode.c | 2 +- fs/sysfs/mount.c | 10 +- fs/sysfs/symlink.c | 12 ++- fs/sysfs/sysfs.h | 1 - include/linux/device.h | 10 ++ include/linux/kobject.h | 25 +++-- include/linux/pm.h | 11 -- kernel/power/Kconfig | 12 -- lib/kobject_uevent.c | 30 ++---- 23 files changed, 253 insertions(+), 301 deletions(-) delete mode 100644 drivers/base/power/runtime.c
---------------
Akinobu Mita (1):
sysfs: avoid kmem_cache_free(NULL)
Alan Stern (2):
PM: Remove deprecated sysfs files
PM: remove deprecated dpm_runtime_* routines
Brandon Philips (1):
Documentation fix devres.txt: lib/iomap.c -> lib/devres.c
Cornelia Huck (1):
Driver core: check return code of sysfs_create_link()
David Brownell (2):
dev_vdbg(), available with -DVERBOSE_DEBUG
dev_vdbg() documentation
Jens Axboe (1):
debugfs: remove rmdir() non-empty complaint
Kay Sievers (1):
Driver core: accept all valid action-strings in uevent-trigger
Paolo Ciarrocchi (1):
HOWTO: Add the knwon_regression URI to the documentation
Tejun Heo (4):
sysfs: fix sysfs root inode nlink accounting
sysfs: make sysfs_init_inode() static
sysfs: kill an extra put in sysfs_create_link() failure path
sysfs: cosmetic clean up on node creation failure paths
PATCH 1/14 debugfs: remove rmdir() non-empty complaint
From: Jens Axboe <[email protected]>
Hi,
This patch kills the pointless debugfs rmdir() printk() when called on a non-empty directory. blktrace will sometimes have to call it a few times when forcefully ending a trace, which polutes the log with pointless warnings.
Rationale:
- It's more code to work-around this "problem" in the debugfs users, and you would have to add code to check for empty directories to do so (or assume that debugfs is using simple_ helpers, but that would be a layering violation).
- Other rmdir() implementations don't complain about something this silly.
--- fs/debugfs/inode.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
fs/debugfs/inode.c +0 -5
@@ -345,11 +345,6 @@ void debugfs_remove(struct dentry *dentry)switch (dentry->d_inode->i_mode &S_IFMT ) {case S_IFDIR : ret =simple_rmdir (parent->d_inode, dentry); -if (ret) -printk (KERN_ERR -"DebugFS rmdir on %s failed : " -"directory not empty.\n" , - dentry->d_name.name);break ;case S_IFLNK :kfree (dentry->d_inode->i_private);
Signature · 1 line
1.5.2.2
PATCH 2/14 Driver core: accept all valid action-strings in uevent-trigger
From: Kay Sievers <[email protected]>
This allows the uevent file to handle any type of uevent action to be triggered by userspace instead of just the "add" uevent.
--- drivers/base/core.c | 23 ++++++++++++++++++++--- include/linux/kobject.h | 25 +++++++++++++++++-------- lib/kobject_uevent.c | 30 ++++++++++-------------------- 3 files changed, 47 insertions(+), 31 deletions(-)
drivers/base/core.c +20 -3
@@ -24,6 +24,8 @@#include "base.h" #include "power/power.h" +extern const char *kobject_actions[]; +int (*platform_notify)(struct device * dev) =NULL ;int (*platform_notify_remove)(struct device * dev) =NULL ;
@@ -303,10 +305,25 @@ out:static ssize_t store_uevent (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) { -if (memcmp (buf,"add" ,3 ) !=0 ) -dev_err (dev,"uevent: unsupported action-string; this will " -"be ignored in a future kernel version" ); +size_t len = count; +enum kobject_action action; + +if (len && buf[len-1 ] =='\n' ) + len--; + +for (action =0 ; action <KOBJ_MAX ; action++) { +if (strncmp (kobject_actions[action], buf, len) !=0 ) +continue ; +if (kobject_actions[action][len] !='\0' ) +continue ; +kobject_uevent (&dev->kobj, action); + goto out; + } + +dev_err (dev,"uevent: unsupported action-string; this will " +"be ignored in a future kernel version\n" );kobject_uevent (&dev->kobj,KOBJ_ADD ); +out:return count; }
include/linux/kobject.h +17 -8
@@ -36,15 +36,24 @@ extern char uevent_helper[];/* counter to tag the uevent, read only except for the kobject core */ extern u64 uevent_seqnum; -/* the actions here must match the proper string in lib/kobject_uevent.c */ -typedef int __bitwise kobject_action_t ; +/* +* The actions here must match the index to the string array +* in lib/kobject_uevent.c +* +* Do not add new actions here without checking with the driver-core +* maintainers. Action strings are not meant to express subsystem +* or device specific properties. In most cases you want to send a +* kobject_uevent_env(kobj, KOBJ_CHANGE, env) with additional event +* specific variables added to the event environment. +*/ enum kobject_action { -KOBJ_ADD = (__force kobject_action_t )0x01 ,/* exclusive to core */ -KOBJ_REMOVE = (__force kobject_action_t )0x02 ,/* exclusive to core */ -KOBJ_CHANGE = (__force kobject_action_t )0x03 ,/* device state change */ -KOBJ_OFFLINE = (__force kobject_action_t )0x04 ,/* device offline */ -KOBJ_ONLINE = (__force kobject_action_t )0x05 ,/* device online */ -KOBJ_MOVE = (__force kobject_action_t )0x06 ,/* device move */ +KOBJ_ADD , +KOBJ_REMOVE , +KOBJ_CHANGE , +KOBJ_MOVE , +KOBJ_ONLINE , +KOBJ_OFFLINE , +KOBJ_MAX };struct kobject {
lib/kobject_uevent.c +10 -20
@@ -33,25 +33,15 @@ static DEFINE_SPINLOCK(sequence_lock);static struct sock *uevent_sock;#endif -static char *action_to_string (enum kobject_action action) -{ -switch (action) { -case KOBJ_ADD : -return "add" ; -case KOBJ_REMOVE : -return "remove" ; -case KOBJ_CHANGE : -return "change" ; -case KOBJ_OFFLINE : -return "offline" ; -case KOBJ_ONLINE : -return "online" ; -case KOBJ_MOVE : -return "move" ; -default : -return NULL ; - } -} +/* the strings here must match the enum in include/linux/kobject.h */ +const char *kobject_actions[] = { +"add" , +"remove" , +"change" , +"move" , +"online" , +"offline" , +};/** * kobject_uevent_env - send an uevent with environmental data
@@ -83,7 +73,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,pr_debug ("%s\n" , __FUNCTION__); - action_string =action_to_string (action); + action_string = kobject_actions[action];if (!action_string) {pr_debug ("kobject attempted to send uevent without action_string!\n" );return -EINVAL ;
Signature · 1 line
1.5.2.2
PATCH 3/14 PM: Remove deprecated sysfs files
From: Alan Stern <[email protected]>
This patch (as932) removes the deprecated sysfs .../power/state attribute files.
--- Documentation/feature-removal-schedule.txt | 3 +- drivers/base/power/sysfs.c | 66 ---------------------------- kernel/power/Kconfig | 12 ----- 3 files changed, 1 insertions(+), 80 deletions(-)
Documentation/feature-removal-schedule.txt +1 -2
@@ -26,8 +26,7 @@ Who: Hans Verkuil <[email protected]> and --------------------------- -What: /sys/devices/.../power/state - dev->power.power_state +What: dev->power.power_state dpm_runtime_{suspend,resume)() When: July 2007 Why: Broken design for runtime control over driver power states, confusing
drivers/base/power/sysfs.c +0 -66
78 lines not on this page · raw message
kernel/power/Kconfig +0 -12
@@ -65,18 +65,6 @@ config PM_TRACE CAUTION: this option will cause your machine's real-time clock to be set to an invalid time after a resume. -config PM_SYSFS_DEPRECATED - bool "Driver model /sys/devices/.../power/state files (DEPRECATED)" - depends on PM && SYSFS - default n - help - The driver model started out with a sysfs file intended to provide - a userspace hook for device power management. This feature has never - worked very well, except for limited testing purposes, and so it will - be removed. It's not clear that a generic mechanism could really - handle the wide variability of device power states; any replacements - are likely to be bus or driver specific. - config SOFTWARE_SUSPEND bool "Software Suspend (Hibernation)" depends on PM && SWAP && (((X86 || PPC64_SWSUSP) && (!SMP || SUSPEND_SMP)) || ((FRV || PPC32) && !SMP))
Signature · 1 line
1.5.2.2
PATCH 4/14 PM: remove deprecated dpm_runtime_* routines
From: Alan Stern <[email protected]>
This patch (as933) removes the deprecated dpm_runtime_suspend() and dpm_runtime_resume() routines from the PM core. The only user of those routines is the PCMCIA ds driver; local replacements are added.
--- Documentation/feature-removal-schedule.txt | 1 - drivers/base/power/Makefile | 2 +- drivers/base/power/power.h | 5 -- drivers/base/power/runtime.c | 85 ---------------------------- drivers/pcmcia/ds.c | 40 +++++++++++-- include/linux/pm.h | 11 ---- 6 files changed, 35 insertions(+), 109 deletions(-) delete mode 100644 drivers/base/power/runtime.c
Documentation/feature-removal-schedule.txt +0 -1
@@ -27,7 +27,6 @@ Who: Hans Verkuil <[email protected]> and --------------------------- What: dev->power.power_state - dpm_runtime_{suspend,resume)() When: July 2007 Why: Broken design for runtime control over driver power states, confusing driver-internal runtime power management with: mechanisms to support
drivers/base/power/Makefile +1 -1
@@ -1,5 +1,5 @@ obj-y := shutdown.o -obj-$(CONFIG_PM) += main.o suspend.o resume.o runtime.o sysfs.o +obj-$(CONFIG_PM) += main.o suspend.o resume.o sysfs.o obj-$(CONFIG_PM_TRACE) += trace.o ifeq ($(CONFIG_DEBUG_DRIVER),y)
drivers/base/power/power.h +0 -5
@@ -62,11 +62,6 @@ extern int resume_device(struct device *);*/ extern int suspend_device (struct device *,pm_message_t ); - -/* -* runtime.c -*/ -#else /* CONFIG_PM */
drivers/base/power/runtime.c deleted +0 -85
85 lines not on this page · raw message
drivers/pcmcia/ds.c +34 -6
@@ -1127,6 +1127,34 @@ static int pcmcia_bus_uevent(struct device *dev, char **envp, int num_envp,#endif +/************************ runtime PM support ***************************/ + +static int pcmcia_dev_suspend (struct device *dev,pm_message_t state); +static int pcmcia_dev_resume (struct device *dev); + +static int runtime_suspend (struct device *dev) +{ +int rc; + +down (&dev->sem); + rc =pcmcia_dev_suspend (dev,PMSG_SUSPEND ); +up (&dev->sem); +if (!rc) + dev->power.power_state.event =PM_EVENT_SUSPEND ; +return rc; +} + +static void runtime_resume (struct device *dev) +{ +int rc; + +down (&dev->sem); + rc =pcmcia_dev_resume (dev); +up (&dev->sem); +if (!rc) + dev->power.power_state.event =PM_EVENT_ON ; +} +/************************ per-device sysfs output ***************************/ #define pcmcia_device_attr (field, test, format) \
@@ -1173,9 +1201,9 @@ static ssize_t pcmcia_store_pm_state(struct device *dev, struct device_attributereturn -EINVAL ;if ((!p_dev->suspended) && !strncmp (buf,"off" ,3 )) - ret =dpm_runtime_suspend (dev,PMSG_SUSPEND ); + ret =runtime_suspend (dev);else if (p_dev->suspended && !strncmp (buf,"on" ,2 )) -dpm_runtime_resume (dev); +runtime_resume (dev);return ret ? ret : count; }
@@ -1312,10 +1340,10 @@ static int pcmcia_bus_suspend_callback(struct device *dev, void * _data)struct pcmcia_socket *skt = _data;struct pcmcia_device *p_dev =to_pcmcia_dev (dev); -if (p_dev->socket != skt) +if (p_dev->socket != skt || p_dev->suspended)return 0 ; -return dpm_runtime_suspend (dev,PMSG_SUSPEND ); +return runtime_suspend (dev); }static int pcmcia_bus_resume_callback (struct device *dev,void * _data)
@@ -1323,10 +1351,10 @@ static int pcmcia_bus_resume_callback(struct device *dev, void * _data)struct pcmcia_socket *skt = _data;struct pcmcia_device *p_dev =to_pcmcia_dev (dev); -if (p_dev->socket != skt) +if (p_dev->socket != skt || !p_dev->suspended)return 0 ; -dpm_runtime_resume (dev); +runtime_resume (dev);return 0 ; }
include/linux/pm.h +0 -11
@@ -284,8 +284,6 @@ extern int device_prepare_suspend(pm_message_t state);#define device_may_wakeup (dev) \ (device_can_wakeup(dev) && (dev)->power.should_wakeup) -extern int dpm_runtime_suspend (struct device *,pm_message_t ); -extern void dpm_runtime_resume (struct device *);extern void __suspend_report_result (const char *function,void *fn,int ret);#define suspend_report_result (fn, ret) \
@@ -317,15 +315,6 @@ static inline int device_suspend(pm_message_t state)#define device_set_wakeup_enable (dev,val) do{}while(0)#define device_may_wakeup (dev) (0) -static inline int dpm_runtime_suspend (struct device * dev,pm_message_t state) -{ -return 0 ; -} - -static inline void dpm_runtime_resume (struct device * dev) -{ -} -#define suspend_report_result (fn, ret) do { } while (0)static inline int call_platform_enable_wakeup (struct device *dev,int is_on)
Signature · 1 line
1.5.2.2
PATCH 5/14 sysfs: avoid kmem_cache_free(NULL)
From: Akinobu Mita <[email protected]>
kmem_cache_free() with NULL is not allowed. But it may happen if out of memory error is triggered in sysfs_new_dirent(). This patch fixes that error handling.
--- fs/sysfs/dir.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)
fs/sysfs/dir.c +7 -6
@@ -361,20 +361,20 @@ static struct dentry_operations sysfs_dentry_ops = {struct sysfs_dirent *sysfs_new_dirent (const char *name,umode_t mode,int type) {char *dup_name =NULL ; -struct sysfs_dirent *sd =NULL ; +struct sysfs_dirent *sd;if (type &SYSFS_COPY_NAME ) { name = dup_name =kstrdup (name,GFP_KERNEL );if (!name) - goto err_out; +return NULL ; } sd =kmem_cache_zalloc (sysfs_dir_cachep,GFP_KERNEL );if (!sd) - goto err_out; + goto err_out1;if (sysfs_alloc_ino (&sd->s_ino)) - goto err_out; + goto err_out2;atomic_set (&sd->s_count,1 );atomic_set (&sd->s_active,0 );
@@ -386,9 +386,10 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)return sd; - err_out: -kfree (dup_name); + err_out2:kmem_cache_free (sysfs_dir_cachep, sd); + err_out1: +kfree (dup_name);return NULL ; }
Signature · 1 line
1.5.2.2
PATCH 6/14 Documentation fix devres.txt: lib/iomap.c -> lib/devres.c
From: Brandon Philips <[email protected]>
--- Documentation/driver-model/devres.txt | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Documentation/driver-model/devres.txt +1 -1
@@ -207,7 +207,7 @@ responsibility. This is usually non-issue because bus ops and resource allocations already do the job. For an example of single-instance devres type, read pcim_iomap_table() -in lib/iomap.c. +in lib/devres.c. All devres interface functions can be called without context if the right gfp mask is given.
Signature · 1 line
1.5.2.2
PATCH 7/14 sysfs: fix sysfs root inode nlink accounting
From: Tejun Heo <[email protected]>
While making sysfs indoes hashed, sysfs root inode was left out. Now that nlink accounting depends on the inode being on the hash, sysfs root inode nlink isn't adjusted properly.
Put sysfs root inode on the inode hash by allocating it using sysfs_get_inode() like other sysfs inodes. While at it, massage comments a bit.
--- fs/sysfs/mount.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
fs/sysfs/mount.c +5 -5
@@ -43,19 +43,19 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent) sb->s_time_gran =1 ; sysfs_sb = sb; - inode =new_inode (sysfs_sb); +/* get root inode, initialize and unlock it */ + inode =sysfs_get_inode (&sysfs_root);if (!inode) {pr_debug ("sysfs: could not get root inode\n" );return -ENOMEM ; } -sysfs_init_inode (&sysfs_root, inode); - inode->i_op = &sysfs_dir_inode_operations; inode->i_fop = &sysfs_dir_operations; -/* directory inodes start off with i_nlink == 2 (for "." entry) */ -inc_nlink (inode); +inc_nlink (inode);/* directory, account for "." */ +unlock_new_inode (inode); +/* instantiate and link root dentry */ root =d_alloc_root (inode);if (!root) {pr_debug ("%s: could not get root dentry!\n" ,__FUNCTION__);
Signature · 1 line
1.5.2.2
PATCH 8/14 sysfs: make sysfs_init_inode() static
From: Tejun Heo <[email protected]>
With sysfs_fill_super() converted to use sysfs_get_inode(), there is no user of sysfs_init_inode() outside of fs/sysfs/inode.c. Make it static.
--- fs/sysfs/inode.c | 2 +- fs/sysfs/sysfs.h | 1 - 2 files changed, 1 insertions(+), 2 deletions(-)
fs/sysfs/inode.c +1 -1
@@ -133,7 +133,7 @@ static inline void set_inode_attr(struct inode * inode, struct iattr * iattr)*/ static struct lock_class_key sysfs_inode_imutex_key; -void sysfs_init_inode (struct sysfs_dirent *sd,struct inode *inode) +static void sysfs_init_inode (struct sysfs_dirent *sd,struct inode *inode) { inode->i_blocks =0 ; inode->i_mapping->a_ops = &sysfs_aops;
fs/sysfs/sysfs.h +0 -1
@@ -71,7 +71,6 @@ extern void sysfs_remove_one(struct sysfs_addrm_cxt *acxt,extern int sysfs_addrm_finish (struct sysfs_addrm_cxt *acxt);extern void sysfs_delete_inode (struct inode *inode); -extern void sysfs_init_inode (struct sysfs_dirent *sd,struct inode *inode);extern struct inode *sysfs_get_inode (struct sysfs_dirent *sd);extern void sysfs_instantiate (struct dentry *dentry,struct inode *inode);
Signature · 1 line
1.5.2.2
PATCH 9/14 dev_vdbg(), available with -DVERBOSE_DEBUG
From: David Brownell <[email protected]>
This defines a dev_vdbg() call, which is enabled with -DVERBOSE_DEBUG. When enabled, dev_vdbg() acts just like dev_dbg(). When disabled, it is a NOP ... just like dev_dbg() without -DDEBUG. The specific code was moved out of a USB patch, but lots of drivers have similar support.
That is, code can now be written to use an additional level of debug output, selected at compile time. Many driver authors have found this idiom to be very useful. A typical usage model is for "normal" debug messages to focus on fault paths and not be very "chatty", so that those messages can be left on during normal operation without much of a performance or syslog load. On the other hand "verbose" messages would be noisy enough that they wouldn't normally be enabled; they might even affect timings enough to change system or driver behavior.
--- drivers/usb/core/driver.c | 7 ------- include/linux/device.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 7 deletions(-)
drivers/usb/core/driver.c +0 -7
@@ -29,13 +29,6 @@#include "hcd.h" #include "usb.h" -#define VERBOSE_DEBUG 0 - -#if VERBOSE_DEBUG -#define dev_vdbg dev_dbg -#else -#define dev_vdbg(dev, fmt, args...) do { } while (0) -#endif #ifdef CONFIG_HOTPLUG
include/linux/device.h +10 -0
@@ -572,6 +572,16 @@ dev_dbg(struct device * dev, const char * fmt, ...) }#endif +#ifdef VERBOSE_DEBUG +#define dev_vdbg dev_dbg +#else +static inline int __attribute__ ((format (printf,2 ,3 ))) +dev_vdbg (struct device * dev,const char * fmt, ...) +{ +return 0 ; +} +#endif + #define dev_err(dev, format, arg...) \ dev_printk(KERN_ERR , dev , format , ## arg) #define dev_info(dev, format, arg...) \
Signature · 1 line
1.5.2.2
PATCH 10/14 dev_vdbg() documentation
From: David Brownell <[email protected]>
Update CodingStyle to talk about "-DDEBUG" message conventions and the new "-DVERBOSE_DEBUG" convention.
--- Documentation/CodingStyle | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-)
Documentation/CodingStyle +18 -2
@@ -633,12 +633,27 @@ covers RTL which is used frequently with assembly language in the kernel. Kernel developers like to be seen as literate. Do mind the spelling of kernel messages to make a good impression. Do not use crippled -words like "dont" and use "do not" or "don't" instead. +words like "dont"; use "do not" or "don't" instead. Make the messages +concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. Printing numbers in parentheses (%d) adds no value and should be avoided. +There are a number of driver model diagnostic macros in <linux/device.h> +which you should use to make sure messages are matched to the right device +and driver, and are tagged with the right level: dev_err(), dev_warn(), +dev_info(), and so forth. For messages that aren't associated with a +particular device, <linux/kernel.h> defines pr_debug() and pr_info(). + +Coming up with good debugging messages can be quite a challenge; and once +you have them, they can be a huge help for remote troubleshooting. Such +messages should be compiled out when the DEBUG symbol is not defined (that +is, by default they are not included). When you use dev_dbg() or pr_debug(), +that's automatic. Many subsystems have Kconfig options to turn on -DDEBUG. +A related convention uses VERBOSE_DEBUG to add dev_vdbg() messages to the +ones already enabled by DEBUG. + Chapter 14: Allocating memory
@@ -790,4 +805,5 @@ Kernel CodingStyle, by [email protected] at OLS 2002: http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/ -- -Last updated on 2006-December-06. +Last updated on 2007-July-13. +
Signature · 1 line
1.5.2.2
PATCH 11/14 HOWTO: Add the knwon_regression URI to the documentation
From: Paolo Ciarrocchi <[email protected]>
We should let everybody know about where the regression list is hosted. The more is known the more it is used.
--- Documentation/HOWTO | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
Documentation/HOWTO +3 -0
@@ -249,6 +249,9 @@ process is as follows: release a new -rc kernel every week. - Process continues until the kernel is considered "ready", the process should last around 6 weeks. + - A list of known regressions present in each -rc release is + tracked at the following URI: + http://kernelnewbies.org/known_regressions It is worth mentioning what Andrew Morton wrote on the linux-kernel mailing list about kernel releases:
Signature · 1 line
1.5.2.2
PATCH 12/14 Driver core: check return code of sysfs_create_link()
From: Cornelia Huck <[email protected]>
Check for return value of sysfs_create_link() in device_add() and device_rename(). Add helper functions device_add_class_symlinks() and device_remove_class_symlinks() to make the code easier to read.
[[email protected]: fix unused var warnings]
--- drivers/base/core.c | 145 +++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 105 insertions(+), 40 deletions(-)
drivers/base/core.c +105 -40
@@ -660,6 +660,82 @@ static int setup_parent(struct device *dev, struct device *parent)return 0 ; } +static int device_add_class_symlinks (struct device *dev) +{ +int error; + +if (!dev->class) +return 0 ; + error =sysfs_create_link (&dev->kobj, &dev->class->subsys.kobj, +"subsystem" ); +if (error) + goto out; +/* +* If this is not a "fake" compatible device, then create the +* symlink from the class to the device. +*/ +if (dev->kobj.parent != &dev->class->subsys.kobj) { + error =sysfs_create_link (&dev->class->subsys.kobj, &dev->kobj, + dev->bus_id); +if (error) + goto out_subsys; + } +/* only bus-device parents get a "device"-link */ +if (dev->parent && dev->parent->bus) { + error =sysfs_create_link (&dev->kobj, &dev->parent->kobj, +"device" ); +if (error) + goto out_busid; +#ifdef CONFIG_SYSFS_DEPRECATED + { +char * class_name =make_class_name (dev->class->name, + &dev->kobj); +if (class_name) + error =sysfs_create_link (&dev->parent->kobj, + &dev->kobj, class_name); +kfree (class_name); +if (error) + goto out_device; + } +#endif + } +return 0 ; + +#ifdef CONFIG_SYSFS_DEPRECATED +out_device: +if (dev->parent) +sysfs_remove_link (&dev->kobj,"device" ); +#endif +out_busid: +if (dev->kobj.parent != &dev->class->subsys.kobj) +sysfs_remove_link (&dev->class->subsys.kobj, dev->bus_id); +out_subsys: +sysfs_remove_link (&dev->kobj,"subsystem" ); +out: +return error; +} + +static void device_remove_class_symlinks (struct device *dev) +{ +if (!dev->class) +return ; +if (dev->parent) { +#ifdef CONFIG_SYSFS_DEPRECATED +char *class_name; + + class_name =make_class_name (dev->class->name, &dev->kobj); +if (class_name) { +sysfs_remove_link (&dev->parent->kobj, class_name); +kfree (class_name); + } +#endif +sysfs_remove_link (&dev->kobj,"device" ); + } +if (dev->kobj.parent != &dev->class->subsys.kobj) +sysfs_remove_link (&dev->class->subsys.kobj, dev->bus_id); +sysfs_remove_link (&dev->kobj,"subsystem" ); +} +/** * device_add - add device to device hierarchy. * @dev: device.
@@ -674,7 +750,6 @@ static int setup_parent(struct device *dev, struct device *parent)int device_add (struct device *dev) {struct device *parent =NULL ; -char *class_name =NULL ;struct class_interface *class_intf;int error = -EINVAL ;
@@ -714,27 +789,9 @@ int device_add(struct device *dev) goto ueventattrError; } -if (dev->class) { -sysfs_create_link (&dev->kobj, &dev->class->subsys.kobj, -"subsystem" ); -/* If this is not a "fake" compatible device, then create the -* symlink from the class to the device. */ -if (dev->kobj.parent != &dev->class->subsys.kobj) -sysfs_create_link (&dev->class->subsys.kobj, - &dev->kobj, dev->bus_id); -if (parent) { -sysfs_create_link (&dev->kobj, &dev->parent->kobj, -"device" ); -#ifdef CONFIG_SYSFS_DEPRECATED - class_name =make_class_name (dev->class->name, - &dev->kobj); -if (class_name) -sysfs_create_link (&dev->parent->kobj, - &dev->kobj, class_name); -#endif - } - } - + error =device_add_class_symlinks (dev); +if (error) + goto SymlinkError; error =device_add_attrs (dev);if (error) goto AttrsError;
@@ -761,7 +818,6 @@ int device_add(struct device *dev)up (&dev->class->sem); } Done: -kfree (class_name);put_device (dev);return error; BusError:
@@ -772,6 +828,8 @@ int device_add(struct device *dev) BUS_NOTIFY_DEL_DEVICE, dev);device_remove_attrs (dev); AttrsError: +device_remove_class_symlinks (dev); + SymlinkError:if (MAJOR (dev->devt))device_remove_file (dev, &devt_attr);
@@ -1156,7 +1214,7 @@ int device_rename(struct device *dev, char *new_name) {char *old_class_name =NULL ;char *new_class_name =NULL ; -char *old_symlink_name =NULL ; +char *old_device_name =NULL ;int error; dev =get_device (dev);
@@ -1170,42 +1228,49 @@ int device_rename(struct device *dev, char *new_name) old_class_name =make_class_name (dev->class->name, &dev->kobj);#endif -if (dev->class) { - old_symlink_name =kmalloc (BUS_ID_SIZE ,GFP_KERNEL ); -if (!old_symlink_name) { - error = -ENOMEM ; - goto out_free_old_class; - } -strlcpy (old_symlink_name, dev->bus_id,BUS_ID_SIZE ); + old_device_name =kmalloc (BUS_ID_SIZE ,GFP_KERNEL ); +if (!old_device_name) { + error = -ENOMEM ; + goto out; } - +strlcpy (old_device_name, dev->bus_id,BUS_ID_SIZE );strlcpy (dev->bus_id, new_name,BUS_ID_SIZE ); error =kobject_rename (&dev->kobj, new_name); +if (error) { +strlcpy (dev->bus_id, old_device_name,BUS_ID_SIZE ); + goto out; + }#ifdef CONFIG_SYSFS_DEPRECATED if (old_class_name) { new_class_name =make_class_name (dev->class->name, &dev->kobj);if (new_class_name) { -sysfs_create_link (&dev->parent->kobj, &dev->kobj, - new_class_name); + error =sysfs_create_link (&dev->parent->kobj, + &dev->kobj, new_class_name); +if (error) + goto out;sysfs_remove_link (&dev->parent->kobj, old_class_name); } }#endif if (dev->class) { -sysfs_remove_link (&dev->class->subsys.kobj, - old_symlink_name); -sysfs_create_link (&dev->class->subsys.kobj, &dev->kobj, - dev->bus_id); +sysfs_remove_link (&dev->class->subsys.kobj, old_device_name); + error =sysfs_create_link (&dev->class->subsys.kobj, &dev->kobj, + dev->bus_id); +if (error) { +/* Uh... how to unravel this if restoring can fail? */ +dev_err (dev,"%s: sysfs_create_symlink failed (%d)\n" , + __FUNCTION__, error); + } } +out:put_device (dev);kfree (new_class_name); -kfree (old_symlink_name); - out_free_old_class:kfree (old_class_name); +kfree (old_device_name);return error; }
Signature · 1 line
1.5.2.2
PATCH 13/14 sysfs: kill an extra put in sysfs_create_link() failure path
From: Tejun Heo <[email protected]>
There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and once more when the symlink is released.
Fix it by making only the symlink node responsible for putting target_sd.
--- fs/sysfs/symlink.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
fs/sysfs/symlink.c +2 -0
@@ -86,7 +86,9 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char sd =sysfs_new_dirent (name,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK );if (!sd) goto out_put; + sd->s_elem.symlink.target_sd = target_sd; + target_sd =NULL ;/* reference is now owned by the symlink */ sysfs_addrm_start (&acxt, parent_sd);
Signature · 1 line
1.5.2.2
PATCH 14/14 sysfs: cosmetic clean up on node creation failure paths
From: Tejun Heo <[email protected]>
Node addition failure is detected by testing return value of sysfs_addfm_finish() which returns the number of added and removed nodes. As the function is called as the last step of addition right on top of error handling block, the if blocks looked like the following.
if (sysfs_addrm_finish(&acxt)) success handling, usually return; /* fall through to error handling */
This is the opposite of usual convention in sysfs and makes the code difficult to understand. This patch inverts the test and makes those blocks look more like others.
--- fs/sysfs/dir.c | 12 +++++++----- fs/sysfs/file.c | 9 +++++---- fs/sysfs/symlink.c | 10 ++++++---- 3 files changed, 18 insertions(+), 13 deletions(-)
fs/sysfs/dir.c +7 -5
@@ -699,17 +699,19 @@ static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,/* link in */ sysfs_addrm_start (&acxt, parent_sd); +if (!sysfs_find_dirent (parent_sd, name)) {sysfs_add_one (&acxt, sd);sysfs_link_sibling (sd); } -if (sysfs_addrm_finish (&acxt)) { - *p_sd = sd; -return 0 ; + +if (!sysfs_addrm_finish (&acxt)) { +sysfs_put (sd); +return -EEXIST ; } -sysfs_put (sd); -return -EEXIST ; + *p_sd = sd; +return 0 ; } int sysfs_create_subdir(struct kobject *kobj, const char *name,
fs/sysfs/file.c +5 -4
@@ -410,11 +410,12 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,sysfs_link_sibling (sd); } -if (sysfs_addrm_finish (&acxt)) -return 0 ; +if (!sysfs_addrm_finish (&acxt)) { +sysfs_put (sd); +return -EEXIST ; + } -sysfs_put (sd); -return -EEXIST ; +return 0 ; }
fs/sysfs/symlink.c +6 -4
@@ -97,11 +97,13 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const charsysfs_link_sibling (sd); } -if (sysfs_addrm_finish (&acxt)) -return 0 ; +if (!sysfs_addrm_finish (&acxt)) { + error = -EEXIST ; + goto out_put; + } + +return 0 ; - error = -EEXIST ; -/* fall through */ out_put:sysfs_put (target_sd);sysfs_put (sd);
Signature · 1 line
1.5.2.2