Skip to contentSkip to conversation

more sysfs and driver core patches for 2.6.22

PATCH New Greg KH 1 participant 15 messages Jul 18, 2007

Subsystems (8)
lore
Share & tools
lore

Greg KH

author
#0 lore raw

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

#1 lore raw

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
index 1d533a2..11be8a3 100644
@@ -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
#2 lore raw

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
index 0455aa7..91a0367 100644
@@ -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
index 06cbf41..aa2fe22 100644
@@ -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
index bd5ecbb..6a80c78 100644
@@ -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
#3 lore raw

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
index d05e624..9cf9d83 100644
@@ -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
index 495b7d4..7332847 100644
@@ -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
#4 lore raw

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
index 9cf9d83..1b5c707 100644
@@ -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
index 91f2309..fff1780 100644
@@ -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
index 2760f25..591a0dd 100644
@@ -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
index 143c6ef..a996071 100644
@@ -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_attribute
                 return -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
index 273781c..2735b7c 100644
@@ -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
#5 lore raw

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
index aee966c..2e6775a 100644
@@ -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
#6 lore raw

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
index 6c8d8f2..8569072 100644
@@ -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
#7 lore raw

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
index 402cc35..60714d0 100644
@@ -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
#8 lore raw

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
index 3756e15..10d1b52 100644
@@ -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
index 6a37f23..6b8c8d7 100644
@@ -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
#9 lore raw

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
index 73c4936..6548574 100644
@@ -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
index be2debe..d9f0a57 100644
@@ -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
#10 lore raw

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
index a667eb1..7f1730f 100644
@@ -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
#11 lore raw

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.

Cc Li Yang <[email protected]>
Cc TripleX Chung <[email protected]>
Cc Maggie Chen <[email protected]>
Cc WANG Cong <[email protected]>
Cc Tsugikazu Shibata <[email protected]>
Cc IKEDA Munehiro <[email protected]>
---
 Documentation/HOWTO |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Documentation/HOWTO +3 -0
index 98e2701..f8cc3f8 100644
@@ -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
#12 lore raw

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
index 91a0367..3599ab2 100644
@@ -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
#13 lore raw

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
index 2f86e04..d056e96 100644
@@ -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
#14 lore raw

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
index 2e6775a..048e605 100644
@@ -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
index cc49799..3e1cc06 100644
@@ -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
index d056e96..4ce687f 100644
@@ -97,11 +97,13 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 		sysfs_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