2009-12-23 19:50:40

by Greg KH

[permalink] [raw]
Subject: [GIT PATCH] driver core patches for 2.6.33-git

Here are some 2.6.33-rc1 patches for the driver core:

Nothing major here:
- fixes for devtmpfs warnings on lockdep and command line
options
- documentation updates
- api change to make attributes const. This is added now so
that subsystem maintainers can queue up patches for .34. This
was submitted now at the recommendation of Stephen Rothwell.
- stable kernel rules clarification in the documentation

Please pull from:
master.kernel.org:/pub/scm/linux/kernel/git/gregkh/driver-core-2.6.git/

All of these patches have been in the linux-next and mm trees.

The patches will be sent as a follow-on to this message to lkml for people
to see.

thanks,

greg k-h

------------

Documentation/driver-model/driver.txt | 4 ++--
Documentation/filesystems/sysfs.txt | 12 ++++++------
Documentation/stable_kernel_rules.txt | 24 ++++++++++++++++++++++--
drivers/base/bus.c | 2 +-
drivers/base/core.c | 16 +++++++++++-----
drivers/base/devtmpfs.c | 19 ++++++++++---------
drivers/base/driver.c | 4 ++--
drivers/base/platform.c | 1 +
fs/super.c | 3 ++-
fs/sysfs/bin.c | 6 ++++--
include/linux/device.h | 12 ++++++------
include/linux/sysfs.h | 9 +++++----
12 files changed, 72 insertions(+), 40 deletions(-)

---------------

Kay Sievers (2):
vfs: get_sb_single() - do not pass options twice
devtmpfs: unlock mutex in case of string allocation error

Michael Hennerich (1):
Driver core: export platform_device_register_data as a GPL symbol

Phil Carmody (4):
Driver core: device_attribute parameters can often be const*
Driver core: bin_attribute parameters can often be const*
Driver core: driver_attribute parameters can often be const*
driver core: Prevent reference to freed memory on error path

Sebastian Andrzej Siewior (1):
Doc/stable rules: add new cherry-pick logic

Thomas Gleixner (2):
devtmpfs: Convert dirlock to a mutex
Driver-core: Fix bogus 0 error return in device_add()


2009-12-23 19:53:16

by Greg KH

[permalink] [raw]
Subject: [PATCH 01/10] devtmpfs: Convert dirlock to a mutex

From: Thomas Gleixner <[email protected]>

devtmpfs has a rw_lock dirlock which serializes delete_path and
create_path.

This code was obviously never tested with the usual set of debugging
facilities enabled. In the dirlock held sections the code calls:

- vfs functions which take mutexes
- kmalloc(, GFP_KERNEL)

In both code pathes the might sleep warning triggers and spams dmesg.

Convert the rw_lock to a mutex. There is no reason why this needs to
be a rwlock.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Kay Sievers <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/devtmpfs.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 50375bb..278371c 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -32,7 +32,7 @@ static int dev_mount = 1;
static int dev_mount;
#endif

-static rwlock_t dirlock;
+static DEFINE_MUTEX(dirlock);

static int __init mount_param(char *str)
{
@@ -93,7 +93,7 @@ static int create_path(const char *nodepath)
{
int err;

- read_lock(&dirlock);
+ mutex_lock(&dirlock);
err = dev_mkdir(nodepath, 0755);
if (err == -ENOENT) {
char *path;
@@ -117,7 +117,7 @@ static int create_path(const char *nodepath)
}
kfree(path);
}
- read_unlock(&dirlock);
+ mutex_unlock(&dirlock);
return err;
}

@@ -229,7 +229,7 @@ static int delete_path(const char *nodepath)
if (!path)
return -ENOMEM;

- write_lock(&dirlock);
+ mutex_lock(&dirlock);
for (;;) {
char *base;

@@ -241,7 +241,7 @@ static int delete_path(const char *nodepath)
if (err)
break;
}
- write_unlock(&dirlock);
+ mutex_unlock(&dirlock);

kfree(path);
return err;
@@ -352,8 +352,6 @@ int __init devtmpfs_init(void)
int err;
struct vfsmount *mnt;

- rwlock_init(&dirlock);
-
err = register_filesystem(&dev_fs_type);
if (err) {
printk(KERN_ERR "devtmpfs: unable to register devtmpfs "
--
1.6.5.7

2009-12-23 19:55:16

by Greg KH

[permalink] [raw]
Subject: [PATCH 02/10] vfs: get_sb_single() - do not pass options twice

From: Kay Sievers <[email protected]>

Filesystem code usually destroys the option buffer while
parsing it. This leads to errors when the same buffer is
passed twice. In case we fill a new superblock do not call
remount.

This is needed to quite a warning that the debugfs code
causes every boot.

Cc: Miklos Szeredi <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
fs/super.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 19eb70b..aff046b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -901,8 +901,9 @@ int get_sb_single(struct file_system_type *fs_type,
return error;
}
s->s_flags |= MS_ACTIVE;
+ } else {
+ do_remount_sb(s, flags, data, 0);
}
- do_remount_sb(s, flags, data, 0);
simple_set_mnt(mnt, s);
return 0;
}
--
1.6.5.7

2009-12-23 19:55:51

by Greg KH

[permalink] [raw]
Subject: [PATCH 03/10] Doc/stable rules: add new cherry-pick logic

From: Sebastian Andrzej Siewior <[email protected]>

- it is possible to submit patches for the stable queue without sending
them directly [email protected]. If the tag (Cc: [email protected]) is
available in the sign-off area than hpa's script will filter them into
the stable mailbox once it hits Linus' tree.
- Patches which require others to be applied first can be also specified.
This was discussued in http://lkml.org/lkml/2009/11/9/474

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Cc: Brandon Philips <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
Documentation/stable_kernel_rules.txt | 24 ++++++++++++++++++++++--
1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/stable_kernel_rules.txt b/Documentation/stable_kernel_rules.txt
index a452227..5effa5b 100644
--- a/Documentation/stable_kernel_rules.txt
+++ b/Documentation/stable_kernel_rules.txt
@@ -26,13 +26,33 @@ Procedure for submitting patches to the -stable tree:

- Send the patch, after verifying that it follows the above rules, to
[email protected].
+ - To have the patch automatically included in the stable tree, add the
+ the tag
+ Cc: [email protected]
+ in the sign-off area. Once the patch is merged it will be applied to
+ the stable tree without anything else needing to be done by the author
+ or subsystem maintainer.
+ - If the patch requires other patches as prerequisites which can be
+ cherry-picked than this can be specified in the following format in
+ the sign-off area:
+
+ Cc: <[email protected]> # .32.x: a1f84a3: sched: Check for idle
+ Cc: <[email protected]> # .32.x: 1b9508f: sched: Rate-limit newidle
+ Cc: <[email protected]> # .32.x: fd21073: sched: Fix affinity logic
+ Cc: <[email protected]> # .32.x
+ Signed-off-by: Ingo Molnar <[email protected]>
+
+ The tag sequence has the meaning of:
+ git cherry-pick a1f84a3
+ git cherry-pick 1b9508f
+ git cherry-pick fd21073
+ git cherry-pick <this commit>
+
- The sender will receive an ACK when the patch has been accepted into the
queue, or a NAK if the patch is rejected. This response might take a few
days, according to the developer's schedules.
- If accepted, the patch will be added to the -stable queue, for review by
other developers and by the relevant subsystem maintainer.
- - If the [email protected] address is added to a patch, when it goes into
- Linus's tree it will automatically be emailed to the stable team.
- Security patches should not be sent to this alias, but instead to the
documented [email protected] address.

--
1.6.5.7

2009-12-23 19:53:17

by Greg KH

[permalink] [raw]
Subject: [PATCH 04/10] Driver core: device_attribute parameters can often be const*

From: Phil Carmody <[email protected]>

Most device_attributes are const, and are begging to be
put in a ro section. However, the create and remove
file interfaces were failing to propagate the const promise
which the only functions they call offer.

Signed-off-by: Phil Carmody <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
Documentation/filesystems/sysfs.txt | 8 ++++----
drivers/base/core.c | 6 ++++--
include/linux/device.h | 4 ++--
3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index b245d52..a4ac2b9 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -91,8 +91,8 @@ struct device_attribute {
const char *buf, size_t count);
};

-int device_create_file(struct device *, struct device_attribute *);
-void device_remove_file(struct device *, struct device_attribute *);
+int device_create_file(struct device *, const struct device_attribute *);
+void device_remove_file(struct device *, const struct device_attribute *);

It also defines this helper for defining device attributes:

@@ -316,8 +316,8 @@ DEVICE_ATTR(_name, _mode, _show, _store);

Creation/Removal:

-int device_create_file(struct device *device, struct device_attribute * attr);
-void device_remove_file(struct device * dev, struct device_attribute * attr);
+int device_create_file(struct device *dev, const struct device_attribute * attr);
+void device_remove_file(struct device *dev, const struct device_attribute * attr);


- bus drivers (include/linux/device.h)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index f1290cb..2fd9e61 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -446,7 +446,8 @@ struct kset *devices_kset;
* @dev: device.
* @attr: device attribute descriptor.
*/
-int device_create_file(struct device *dev, struct device_attribute *attr)
+int device_create_file(struct device *dev,
+ const struct device_attribute *attr)
{
int error = 0;
if (dev)
@@ -459,7 +460,8 @@ int device_create_file(struct device *dev, struct device_attribute *attr)
* @dev: device.
* @attr: device attribute descriptor.
*/
-void device_remove_file(struct device *dev, struct device_attribute *attr)
+void device_remove_file(struct device *dev,
+ const struct device_attribute *attr)
{
if (dev)
sysfs_remove_file(&dev->kobj, &attr->attr);
diff --git a/include/linux/device.h b/include/linux/device.h
index 2a73d9b..aa5b3e6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -319,9 +319,9 @@ struct device_attribute {
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)

extern int __must_check device_create_file(struct device *device,
- struct device_attribute *entry);
+ const struct device_attribute *entry);
extern void device_remove_file(struct device *dev,
- struct device_attribute *attr);
+ const struct device_attribute *attr);
extern int __must_check device_create_bin_file(struct device *dev,
struct bin_attribute *attr);
extern void device_remove_bin_file(struct device *dev,
--
1.6.5.7

2009-12-23 19:56:09

by Greg KH

[permalink] [raw]
Subject: [PATCH 05/10] Driver core: bin_attribute parameters can often be const*

From: Phil Carmody <[email protected]>

Many struct bin_attribute descriptors are purely read-only
structures, and there's no need to change them. Therefore
make the promise not to, which will let those descriptors
be put in a ro section.

Signed-off-by: Phil Carmody <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/core.c | 6 ++++--
fs/sysfs/bin.c | 6 ++++--
include/linux/device.h | 6 +++---
include/linux/sysfs.h | 9 +++++----
4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2fd9e61..83afc8b 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -472,7 +472,8 @@ void device_remove_file(struct device *dev,
* @dev: device.
* @attr: device binary attribute descriptor.
*/
-int device_create_bin_file(struct device *dev, struct bin_attribute *attr)
+int device_create_bin_file(struct device *dev,
+ const struct bin_attribute *attr)
{
int error = -EINVAL;
if (dev)
@@ -486,7 +487,8 @@ EXPORT_SYMBOL_GPL(device_create_bin_file);
* @dev: device.
* @attr: device binary attribute descriptor.
*/
-void device_remove_bin_file(struct device *dev, struct bin_attribute *attr)
+void device_remove_bin_file(struct device *dev,
+ const struct bin_attribute *attr)
{
if (dev)
sysfs_remove_bin_file(&dev->kobj, attr);
diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 60c702b..a0a500a 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -483,7 +483,8 @@ void unmap_bin_file(struct sysfs_dirent *attr_sd)
* @attr: attribute descriptor.
*/

-int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
+int sysfs_create_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
{
BUG_ON(!kobj || !kobj->sd || !attr);

@@ -497,7 +498,8 @@ int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
* @attr: attribute descriptor.
*/

-void sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
+void sysfs_remove_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr)
{
sysfs_hash_and_remove(kobj->sd, attr->attr.name);
}
diff --git a/include/linux/device.h b/include/linux/device.h
index aa5b3e6..10d74ce 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -319,13 +319,13 @@ struct device_attribute {
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)

extern int __must_check device_create_file(struct device *device,
- const struct device_attribute *entry);
+ const struct device_attribute *entry);
extern void device_remove_file(struct device *dev,
const struct device_attribute *attr);
extern int __must_check device_create_bin_file(struct device *dev,
- struct bin_attribute *attr);
+ const struct bin_attribute *attr);
extern void device_remove_bin_file(struct device *dev,
- struct bin_attribute *attr);
+ const struct bin_attribute *attr);
extern int device_schedule_callback_owner(struct device *dev,
void (*func)(struct device *dev), struct module *owner);

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 9d68fed..cfa8308 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -99,8 +99,9 @@ int __must_check sysfs_chmod_file(struct kobject *kobj, struct attribute *attr,
void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr);

int __must_check sysfs_create_bin_file(struct kobject *kobj,
- struct bin_attribute *attr);
-void sysfs_remove_bin_file(struct kobject *kobj, struct bin_attribute *attr);
+ const struct bin_attribute *attr);
+void sysfs_remove_bin_file(struct kobject *kobj,
+ const struct bin_attribute *attr);

int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
const char *name);
@@ -175,13 +176,13 @@ static inline void sysfs_remove_file(struct kobject *kobj,
}

static inline int sysfs_create_bin_file(struct kobject *kobj,
- struct bin_attribute *attr)
+ const struct bin_attribute *attr)
{
return 0;
}

static inline void sysfs_remove_bin_file(struct kobject *kobj,
- struct bin_attribute *attr)
+ const struct bin_attribute *attr)
{
}

--
1.6.5.7

2009-12-23 19:56:35

by Greg KH

[permalink] [raw]
Subject: [PATCH 06/10] Driver core: driver_attribute parameters can often be const*

From: Phil Carmody <[email protected]>

Many struct driver_attribute descriptors are purely read-only
structures, and there's no need to change them. Therefore make
the promise not to, which will let those descriptors be put in
a ro section.

Signed-off-by: Phil Carmody <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
Documentation/driver-model/driver.txt | 4 ++--
Documentation/filesystems/sysfs.txt | 4 ++--
drivers/base/driver.c | 4 ++--
include/linux/device.h | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-model/driver.txt b/Documentation/driver-model/driver.txt
index 60120fb..d2cd6fb 100644
--- a/Documentation/driver-model/driver.txt
+++ b/Documentation/driver-model/driver.txt
@@ -226,5 +226,5 @@ struct driver_attribute driver_attr_debug;
This can then be used to add and remove the attribute from the
driver's directory using:

-int driver_create_file(struct device_driver *, struct driver_attribute *);
-void driver_remove_file(struct device_driver *, struct driver_attribute *);
+int driver_create_file(struct device_driver *, const struct driver_attribute *);
+void driver_remove_file(struct device_driver *, const struct driver_attribute *);
diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index a4ac2b9..931c806 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -358,7 +358,7 @@ DRIVER_ATTR(_name, _mode, _show, _store)

Creation/Removal:

-int driver_create_file(struct device_driver *, struct driver_attribute *);
-void driver_remove_file(struct device_driver *, struct driver_attribute *);
+int driver_create_file(struct device_driver *, const struct driver_attribute *);
+void driver_remove_file(struct device_driver *, const struct driver_attribute *);


diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index f367885..90c9fff 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -98,7 +98,7 @@ EXPORT_SYMBOL_GPL(driver_find_device);
* @attr: driver attribute descriptor.
*/
int driver_create_file(struct device_driver *drv,
- struct driver_attribute *attr)
+ const struct driver_attribute *attr)
{
int error;
if (drv)
@@ -115,7 +115,7 @@ EXPORT_SYMBOL_GPL(driver_create_file);
* @attr: driver attribute descriptor.
*/
void driver_remove_file(struct device_driver *drv,
- struct driver_attribute *attr)
+ const struct driver_attribute *attr)
{
if (drv)
sysfs_remove_file(&drv->p->kobj, &attr->attr);
diff --git a/include/linux/device.h b/include/linux/device.h
index 10d74ce..a62799f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -166,9 +166,9 @@ struct driver_attribute driver_attr_##_name = \
__ATTR(_name, _mode, _show, _store)

extern int __must_check driver_create_file(struct device_driver *driver,
- struct driver_attribute *attr);
+ const struct driver_attribute *attr);
extern void driver_remove_file(struct device_driver *driver,
- struct driver_attribute *attr);
+ const struct driver_attribute *attr);

extern int __must_check driver_add_kobj(struct device_driver *drv,
struct kobject *kobj,
--
1.6.5.7

2009-12-23 19:53:21

by Greg KH

[permalink] [raw]
Subject: [PATCH 07/10] Driver-core: Fix bogus 0 error return in device_add()

From: Thomas Gleixner <[email protected]>

If device_add() is called with a device which does not have dev->p set
up, then device_private_init() is called. If that succeeds, then the
error variable is set to 0. Now if the dev_name(dev) check further
down fails, then device_add() correctly terminates, but returns 0.
That of course lets the driver progress. If later another driver uses
this half set up device as parent then device_add() of the child
device explodes and renders sysfs completely unusable.

Set the error to -EINVAL if dev_name() check fails.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Kay Sievers <[email protected]>
Cc: "Hans J. Koch" <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/core.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 83afc8b..2820257 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -909,8 +909,10 @@ int device_add(struct device *dev)
dev->init_name = NULL;
}

- if (!dev_name(dev))
+ if (!dev_name(dev)) {
+ error = -EINVAL;
goto name_error;
+ }

pr_debug("device: '%s': %s\n", dev_name(dev), __func__);

--
1.6.5.7

2009-12-23 19:56:42

by Greg KH

[permalink] [raw]
Subject: [PATCH 08/10] driver core: Prevent reference to freed memory on error path

From: Phil Carmody <[email protected]>

priv is drv->p. So only free drv->p after we've finished using priv.

Found using a static code analysis tool

Signed-off-by: Phil Carmody <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/bus.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 63c143e..c0c5a43 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -703,9 +703,9 @@ int bus_add_driver(struct device_driver *drv)
return 0;

out_unregister:
+ kobject_put(&priv->kobj);
kfree(drv->p);
drv->p = NULL;
- kobject_put(&priv->kobj);
out_put_bus:
bus_put(bus);
return error;
--
1.6.5.7

2009-12-23 19:53:20

by Greg KH

[permalink] [raw]
Subject: [PATCH 09/10] Driver core: export platform_device_register_data as a GPL symbol

From: Michael Hennerich <[email protected]>

This allows MFD's to register/bind drivers for their sub devices while
still being compiled as a module.

Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/platform.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 9d2ee25..58efaf2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -441,6 +441,7 @@ error:
platform_device_put(pdev);
return ERR_PTR(retval);
}
+EXPORT_SYMBOL_GPL(platform_device_register_data);

static int platform_drv_probe(struct device *_dev)
{
--
1.6.5.7

2009-12-23 19:55:38

by Greg KH

[permalink] [raw]
Subject: [PATCH 10/10] devtmpfs: unlock mutex in case of string allocation error

From: Kay Sievers <[email protected]>

Reported-by: Kirill A. Shutemov <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/devtmpfs.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 278371c..090dd48 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -101,8 +101,10 @@ static int create_path(const char *nodepath)

/* parent directories do not exist, create them */
path = kstrdup(nodepath, GFP_KERNEL);
- if (!path)
- return -ENOMEM;
+ if (!path) {
+ err = -ENOMEM;
+ goto out;
+ }
s = path;
for (;;) {
s = strchr(s, '/');
@@ -117,6 +119,7 @@ static int create_path(const char *nodepath)
}
kfree(path);
}
+out:
mutex_unlock(&dirlock);
return err;
}
--
1.6.5.7

2009-12-27 12:36:20

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 02/10] vfs: get_sb_single() - do not pass options twice

Hi,

Greg Kroah-Hartman <[email protected]> writes:

> From: Kay Sievers <[email protected]>
>
> Filesystem code usually destroys the option buffer while
> parsing it. This leads to errors when the same buffer is
> passed twice. In case we fill a new superblock do not call
> remount.
>
> This is needed to quite a warning that the debugfs code
> causes every boot.
>
> Cc: Miklos Szeredi <[email protected]>
> Signed-off-by: Kay Sievers <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> fs/super.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 19eb70b..aff046b 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -901,8 +901,9 @@ int get_sb_single(struct file_system_type *fs_type,
> return error;
> }
> s->s_flags |= MS_ACTIVE;
> + } else {
> + do_remount_sb(s, flags, data, 0);
> }
> - do_remount_sb(s, flags, data, 0);
> simple_set_mnt(mnt, s);
> return 0;
> }

This breaks the historical behavior. Several users of get_sb_single() is
parse data only on ->remount_fs. Well, ok, I like new behavior actually.
But we need to convert to new behavior such users.

I've listed all possibly affected users up (if I'm not missing). This
means, using both data on ->fill_super and ->remount_fs is devtmpfs
only. And capifs, usbfs, devpts would be needed the patch.

arch/powerpc/platforms/cell/spufs/inode.c
->fill_super
arch/s390/hypfs/inode.c
->fill_super
drivers/isdn/capi/capifs.c
->remount_fs
drivers/base/devtmpfs.c
->fill_super
->remount_fs
drivers/usb/core/inode.c
->remount_fs
fs/devpts/inode.c
->remount_fs

Currently, I'm working on other bugs, so I just attached quick fix for
regression.
--
OGAWA Hirofumi <[email protected]>



Signed-off-by: OGAWA Hirofumi <[email protected]>
---

fs/super.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff -puN fs/super.c~get_sb_single-fix fs/super.c
--- linux-2.6/fs/super.c~get_sb_single-fix 2009-12-27 20:48:07.000000000 +0900
+++ linux-2.6-hirofumi/fs/super.c 2009-12-27 20:48:29.000000000 +0900
@@ -912,9 +912,8 @@ int get_sb_single(struct file_system_typ
return error;
}
s->s_flags |= MS_ACTIVE;
- } else {
- do_remount_sb(s, flags, data, 0);
}
+ do_remount_sb(s, flags, data, 0);
simple_set_mnt(mnt, s);
return 0;
}
_

2009-12-30 18:06:16

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH 02/10] vfs: get_sb_single() - do not pass options twice

On Sun, Dec 27, 2009 at 13:36, OGAWA Hirofumi
<[email protected]> wrote:
>> Filesystem code usually destroys the option buffer while
>> parsing it. This leads to errors when the same buffer is
>> passed twice. In case we fill a new superblock do not call
>> remount.

> This breaks the historical behavior. Several users of get_sb_single() is
> parse data only on ->remount_fs. Well, ok, I like new behavior actually.
> But we need to convert to new behavior such users.
>
> I've listed all possibly affected users up (if I'm not missing). This
> means, using both data on ->fill_super and ->remount_fs is devtmpfs
> only. And capifs, usbfs, devpts would be needed the patch.

Hmm, these filesystem are probably not going to overwrite their own
default options with their own special parameters to parse when they
allocate their superblock. That would be pretty weird, wouldn't it?
Seems they currently don't even pass "data" pointer around at that
time.

It's a bit different with tmpfs as we use it as a generic backend for
a special purpose filesystem.

What's the issue you are seeing, or have in mind?

Thanks,
Kay

2009-12-30 19:30:22

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 02/10] vfs: get_sb_single() - do not pass options twice

Kay Sievers <[email protected]> writes:

> On Sun, Dec 27, 2009 at 13:36, OGAWA Hirofumi
> <[email protected]> wrote:
>>> Filesystem code usually destroys the option buffer while
>>> parsing it. This leads to errors when the same buffer is
>>> passed twice. In case we fill a new superblock do not call
>>> remount.
>
>> This breaks the historical behavior. Several users of get_sb_single() is
>> parse data only on ->remount_fs. Well, ok, I like new behavior actually.
>> But we need to convert to new behavior such users.
>>
>> I've listed all possibly affected users up (if I'm not missing). This
>> means, using both data on ->fill_super and ->remount_fs is devtmpfs
>> only. And capifs, usbfs, devpts would be needed the patch.
>
> Hmm, these filesystem are probably not going to overwrite their own
> default options with their own special parameters to parse when they
> allocate their superblock. That would be pretty weird, wouldn't it?
> Seems they currently don't even pass "data" pointer around at that
> time.

Oops, I was missing that those all fs was using kern_mount(). Sorry for
noise.
--
OGAWA Hirofumi <[email protected]>