Kay pointed out to me the other day that we are dragging around 20 bytes
in every struct kobject and in every struct device to contain a name
string that can be dynamically allocated instead. For small device
names (the majority), this savings can add up, especially with a lot of
individual devices.
So, I started out by getting rid of the static array in the kobject
structure, as we already were dynamically allocating space if it was
needed.
Of course, this required a number of other minor cleanups through the
code tree to handle places where we were incorrectly directly accessing
the kobject name instead of using the "proper" function. I also got
sidetracked by a few driver core and kobject.h cleanups of macros that
are no longer needed, or functions that no longer need to be global
(they were never exported, so we don't have to worry about that mess...)
And I added a change to trigger a warning if we add an attribute to
sysfs that we have already had created, to help the SCSI developers out
with their driver model reworks.
So, here's a series of 11 patches that I've added to my tree, and will
send to Linus when 2.6.24 is opened up.
Any review comments are appreciated. The full diffstat is below showing
that overall, we did get rid of more code than was added.
thanks,
greg k-h
block/bsg.c | 5 --
block/elevator.c | 2
block/ll_rw_blk.c | 2
drivers/acpi/bus.c | 2
drivers/base/base.h | 2
drivers/base/bus.c | 60 ++++++++++++++--------------
drivers/base/class.c | 6 +-
drivers/base/sys.c | 2
drivers/char/raw.c | 5 --
drivers/cpufreq/cpufreq.c | 2
drivers/edac/edac_mc_sysfs.c | 3 -
drivers/md/md.c | 3 -
drivers/media/dvb/dvb-core/dvbdev.c | 5 --
drivers/usb/core/devio.c | 6 --
fs/dlm/lockspace.c | 2
fs/gfs2/locking/dlm/sysfs.c | 2
fs/gfs2/sys.c | 2
fs/ocfs2/cluster/masklog.c | 3 -
fs/partitions/check.c | 12 +++--
fs/sysfs/dir.c | 6 ++
include/linux/kobject.h | 46 +---------------------
lib/kobject.c | 75 ++++++++++++++++--------------------
net/bridge/br_sysfs_br.c | 2
23 files changed, 102 insertions(+), 153 deletions(-)
get_bus() should not be globally visable as it is not used by anything
other than drivers/base/bus.c. This patch removes the visability of it,
and renames it to match all of the other *_get() functions in the
kernel.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/base.h | 1 -
drivers/base/bus.c | 24 ++++++++++++------------
2 files changed, 12 insertions(+), 13 deletions(-)
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -18,7 +18,6 @@ extern int attribute_container_init(void
extern int bus_add_device(struct device * dev);
extern void bus_attach_device(struct device * dev);
extern void bus_remove_device(struct device * dev);
-extern struct bus_type *get_bus(struct bus_type * bus);
extern int bus_add_driver(struct device_driver *);
extern void bus_remove_driver(struct device_driver *);
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -30,6 +30,12 @@
static int __must_check bus_rescan_devices_helper(struct device *dev,
void *data);
+static struct bus_type *bus_get(struct bus_type *bus)
+{
+ return bus ? container_of(kset_get(&bus->subsys),
+ struct bus_type, subsys) : NULL;
+}
+
static void bus_put(struct bus_type *bus)
{
kset_put(&bus->subsys);
@@ -127,7 +133,7 @@ static struct sysfs_ops bus_sysfs_ops =
int bus_create_file(struct bus_type * bus, struct bus_attribute * attr)
{
int error;
- if (get_bus(bus)) {
+ if (bus_get(bus)) {
error = sysfs_create_file(&bus->subsys.kobj, &attr->attr);
bus_put(bus);
} else
@@ -137,7 +143,7 @@ int bus_create_file(struct bus_type * bu
void bus_remove_file(struct bus_type * bus, struct bus_attribute * attr)
{
- if (get_bus(bus)) {
+ if (bus_get(bus)) {
sysfs_remove_file(&bus->subsys.kobj, &attr->attr);
bus_put(bus);
}
@@ -177,7 +183,7 @@ static int driver_helper(struct device *
static ssize_t driver_unbind(struct device_driver *drv,
const char *buf, size_t count)
{
- struct bus_type *bus = get_bus(drv->bus);
+ struct bus_type *bus = bus_get(drv->bus);
struct device *dev;
int err = -ENODEV;
@@ -204,7 +210,7 @@ static DRIVER_ATTR(unbind, S_IWUSR, NULL
static ssize_t driver_bind(struct device_driver *drv,
const char *buf, size_t count)
{
- struct bus_type *bus = get_bus(drv->bus);
+ struct bus_type *bus = bus_get(drv->bus);
struct device *dev;
int err = -ENODEV;
@@ -435,7 +441,7 @@ static inline void remove_deprecated_bus
*/
int bus_add_device(struct device * dev)
{
- struct bus_type * bus = get_bus(dev->bus);
+ struct bus_type * bus = bus_get(dev->bus);
int error = 0;
if (bus) {
@@ -611,7 +617,7 @@ static inline void remove_probe_files(st
*/
int bus_add_driver(struct device_driver *drv)
{
- struct bus_type * bus = get_bus(drv->bus);
+ struct bus_type * bus = bus_get(drv->bus);
int error = 0;
if (!bus)
@@ -731,12 +737,6 @@ int device_reprobe(struct device *dev)
}
EXPORT_SYMBOL_GPL(device_reprobe);
-struct bus_type *get_bus(struct bus_type *bus)
-{
- return bus ? container_of(kset_get(&bus->subsys),
- struct bus_type, subsys) : NULL;
-}
-
/**
* find_bus - locate bus by name.
* @name: name of bus.
This macro is only used by the driver core and is held over from when we
had subsystems. It is not needed anymore.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/sys.c | 2 +-
include/linux/kobject.h | 14 --------------
2 files changed, 1 insertion(+), 15 deletions(-)
--- a/drivers/base/sys.c
+++ b/drivers/base/sys.c
@@ -139,7 +139,7 @@ int sysdev_class_register(struct sysdev_
kobject_name(&cls->kset.kobj));
INIT_LIST_HEAD(&cls->drivers);
cls->kset.kobj.parent = &system_subsys.kobj;
- kset_set_kset_s(cls, system_subsys);
+ cls->kset.kobj.kset = &system_subsys;
return kset_register(&cls->kset);
}
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -227,20 +227,6 @@ extern struct kset hypervisor_subsys;
#define kobj_set_kset_s(obj,subsys) \
(obj)->kobj.kset = &(subsys)
-/**
- * kset_set_kset_s(obj,subsys) - set kset for embedded kset.
- * @obj: ptr to some object type.
- * @subsys: a subsystem object (not a ptr).
- *
- * Can be used for any object type with an embedded ->kset.
- * Sets the kset of @obj's embedded kobject (via its embedded
- * kset) to @subsys.kset. This makes @obj a member of that
- * kset.
- */
-
-#define kset_set_kset_s(obj,subsys) \
- (obj)->kset.kobj.kset = &(subsys)
-
extern void subsystem_init(struct kset *);
extern int __must_check subsystem_register(struct kset *);
extern void subsystem_unregister(struct kset *);
put_bus() should not be globally visable as it is not used by anything
other than drivers/base/bus.c. This patch removes the visability of it,
and renames it to match all of the other *_put() functions in the
kernel.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/base.h | 1 -
drivers/base/bus.c | 29 ++++++++++++++---------------
2 files changed, 14 insertions(+), 16 deletions(-)
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -19,7 +19,6 @@ extern int bus_add_device(struct device
extern void bus_attach_device(struct device * dev);
extern void bus_remove_device(struct device * dev);
extern struct bus_type *get_bus(struct bus_type * bus);
-extern void put_bus(struct bus_type * bus);
extern int bus_add_driver(struct device_driver *);
extern void bus_remove_driver(struct device_driver *);
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -30,6 +30,11 @@
static int __must_check bus_rescan_devices_helper(struct device *dev,
void *data);
+static void bus_put(struct bus_type *bus)
+{
+ kset_put(&bus->subsys);
+}
+
static ssize_t
drv_attr_show(struct kobject * kobj, struct attribute * attr, char * buf)
{
@@ -124,7 +129,7 @@ int bus_create_file(struct bus_type * bu
int error;
if (get_bus(bus)) {
error = sysfs_create_file(&bus->subsys.kobj, &attr->attr);
- put_bus(bus);
+ bus_put(bus);
} else
error = -EINVAL;
return error;
@@ -134,7 +139,7 @@ void bus_remove_file(struct bus_type * b
{
if (get_bus(bus)) {
sysfs_remove_file(&bus->subsys.kobj, &attr->attr);
- put_bus(bus);
+ bus_put(bus);
}
}
@@ -186,7 +191,7 @@ static ssize_t driver_unbind(struct devi
err = count;
}
put_device(dev);
- put_bus(bus);
+ bus_put(bus);
return err;
}
static DRIVER_ATTR(unbind, S_IWUSR, NULL, driver_unbind);
@@ -219,7 +224,7 @@ static ssize_t driver_bind(struct device
err = -ENODEV;
}
put_device(dev);
- put_bus(bus);
+ bus_put(bus);
return err;
}
static DRIVER_ATTR(bind, S_IWUSR, NULL, driver_bind);
@@ -459,7 +464,7 @@ out_subsys:
out_id:
device_remove_attrs(bus, dev);
out_put:
- put_bus(dev->bus);
+ bus_put(dev->bus);
return error;
}
@@ -509,7 +514,7 @@ void bus_remove_device(struct device * d
}
pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
device_release_driver(dev);
- put_bus(dev->bus);
+ bus_put(dev->bus);
}
}
@@ -646,7 +651,7 @@ int bus_add_driver(struct device_driver
out_unregister:
kobject_unregister(&drv->kobj);
out_put_bus:
- put_bus(bus);
+ bus_put(bus);
return error;
}
@@ -671,7 +676,7 @@ void bus_remove_driver(struct device_dri
driver_detach(drv);
module_remove_driver(drv);
kobject_unregister(&drv->kobj);
- put_bus(drv->bus);
+ bus_put(drv->bus);
}
@@ -732,12 +737,6 @@ struct bus_type *get_bus(struct bus_type
struct bus_type, subsys) : NULL;
}
-void put_bus(struct bus_type * bus)
-{
- kset_put(&bus->subsys);
-}
-
-
/**
* find_bus - locate bus by name.
* @name: name of bus.
@@ -874,7 +873,7 @@ out:
* @bus: bus.
*
* Unregister the child subsystems and the bus itself.
- * Finally, we call put_bus() to release the refcount
+ * Finally, we call bus_put() to release the refcount
*/
void bus_unregister(struct bus_type * bus)
{
There are no more subsystems, it's a kset now so remove the function and
the only two users, which are in the driver core.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/bus.c | 2 +-
drivers/base/class.c | 2 +-
include/linux/kobject.h | 7 -------
lib/kobject.c | 2 +-
4 files changed, 3 insertions(+), 10 deletions(-)
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -728,7 +728,7 @@ EXPORT_SYMBOL_GPL(device_reprobe);
struct bus_type *get_bus(struct bus_type *bus)
{
- return bus ? container_of(subsys_get(&bus->subsys),
+ return bus ? container_of(kset_get(&bus->subsys),
struct bus_type, subsys) : NULL;
}
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -93,7 +93,7 @@ void class_remove_file(struct class * cl
static struct class *class_get(struct class *cls)
{
if (cls)
- return container_of(subsys_get(&cls->subsys), struct class, subsys);
+ return container_of(kset_get(&cls->subsys), struct class, subsys);
return NULL;
}
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -231,13 +231,6 @@ extern void subsystem_init(struct kset *
extern int __must_check subsystem_register(struct kset *);
extern void subsystem_unregister(struct kset *);
-static inline struct kset *subsys_get(struct kset *s)
-{
- if (s)
- return kset_get(s);
- return NULL;
-}
-
struct subsys_attribute {
struct attribute attr;
ssize_t (*show)(struct kset *, char *);
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -679,7 +679,7 @@ int subsys_create_file(struct kset *s, s
if (!s || !a)
return -EINVAL;
- if (subsys_get(s)) {
+ if (kset_get(s)) {
error = sysfs_create_file(&s->kobj, &a->attr);
kset_put(s);
}
There are no more subsystems, it's a kset now so remove the function and
the only two users, which are in the driver core.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/bus.c | 2 +-
drivers/base/class.c | 2 +-
include/linux/kobject.h | 5 -----
lib/kobject.c | 2 +-
4 files changed, 3 insertions(+), 8 deletions(-)
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -734,7 +734,7 @@ struct bus_type *get_bus(struct bus_type
void put_bus(struct bus_type * bus)
{
- subsys_put(&bus->subsys);
+ kset_put(&bus->subsys);
}
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -100,7 +100,7 @@ static struct class *class_get(struct cl
static void class_put(struct class * cls)
{
if (cls)
- subsys_put(&cls->subsys);
+ kset_put(&cls->subsys);
}
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -238,11 +238,6 @@ static inline struct kset *subsys_get(st
return NULL;
}
-static inline void subsys_put(struct kset *s)
-{
- kset_put(s);
-}
-
struct subsys_attribute {
struct attribute attr;
ssize_t (*show)(struct kset *, char *);
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -681,7 +681,7 @@ int subsys_create_file(struct kset *s, s
if (subsys_get(s)) {
error = sysfs_create_file(&s->kobj, &a->attr);
- subsys_put(s);
+ kset_put(s);
}
return error;
}
This macro is only used by the driver core and is held over from when we
had subsystems. It is not needed anymore.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/bus.c | 3 ++-
drivers/base/class.c | 2 +-
include/linux/kobject.h | 13 -------------
3 files changed, 3 insertions(+), 15 deletions(-)
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -823,7 +823,8 @@ int bus_register(struct bus_type * bus)
if (retval)
goto out;
- subsys_set_kset(bus, bus_subsys);
+ bus->subsys.kobj.kset = &bus_subsys;
+
retval = subsystem_register(&bus->subsys);
if (retval)
goto out;
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -149,7 +149,7 @@ int class_register(struct class * cls)
if (error)
return error;
- subsys_set_kset(cls, class_subsys);
+ cls->subsys.kobj.kset = &class_subsys;
error = subsystem_register(&cls->subsys);
if (!error) {
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -241,19 +241,6 @@ extern struct kset hypervisor_subsys;
#define kset_set_kset_s(obj,subsys) \
(obj)->kset.kobj.kset = &(subsys)
-/**
- * subsys_set_kset(obj,subsys) - set kset for subsystem
- * @obj: ptr to some object type.
- * @_subsys: a subsystem object (not a ptr).
- *
- * Can be used for any object type with an embedded ->subsys.
- * Sets the kset of @obj's kobject to @subsys.kset. This makes
- * the object a member of that kset.
- */
-
-#define subsys_set_kset(obj,_subsys) \
- (obj)->subsys.kobj.kset = &(_subsys)
-
extern void subsystem_init(struct kset *);
extern int __must_check subsystem_register(struct kset *);
extern void subsystem_unregister(struct kset *);
We want to let people know when we create a duplicate sysfs file, as
they need to fix up their code.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
fs/sysfs/dir.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -428,8 +428,12 @@ void sysfs_addrm_start(struct sysfs_addr
*/
int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
{
- if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
+ if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
+ printk(KERN_WARNING, "sysfs: duplicate filename '%s' "
+ "can not be created\n", sd->s_name);
+ WARN_ON(1);
return -EEXIST;
+ }
sd->s_parent = sysfs_get(acxt->parent_sd);
A kset should not have its name set directly, so dynamically set the
name at runtime.
This is needed to remove the static array in the kobject structure which
will be changed in a future patch.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/edac/edac_mc_sysfs.c | 3 ++-
fs/dlm/lockspace.c | 2 +-
fs/gfs2/locking/dlm/sysfs.c | 2 +-
fs/gfs2/sys.c | 2 +-
fs/ocfs2/cluster/masklog.c | 3 ++-
5 files changed, 7 insertions(+), 5 deletions(-)
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -743,7 +743,7 @@ static struct kobj_type ktype_mc_set_att
* /sys/devices/system/edac/mc
*/
static struct kset mc_kset = {
- .kobj = {.name = "mc", .ktype = &ktype_mc_set_attribs },
+ .kobj = {.ktype = &ktype_mc_set_attribs },
.ktype = &ktype_mci,
};
@@ -1010,6 +1010,7 @@ int edac_sysfs_setup_mc_kset(void)
}
/* Init the MC's kobject */
+ kobject_set_name(&mc_kset.kobj, "mc");
mc_kset.kobj.parent = &edac_class->kset.kobj;
/* register the mc_kset */
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -167,7 +167,6 @@ static struct kobj_type dlm_ktype = {
};
static struct kset dlm_kset = {
- .kobj = {.name = "dlm",},
.ktype = &dlm_ktype,
};
@@ -228,6 +227,7 @@ int dlm_lockspace_init(void)
INIT_LIST_HEAD(&lslist);
spin_lock_init(&lslist_lock);
+ kobject_set_name(&dlm_kset.kobj, "dlm");
kobj_set_kset_s(&dlm_kset, kernel_subsys);
error = kset_register(&dlm_kset);
if (error)
--- a/fs/gfs2/locking/dlm/sysfs.c
+++ b/fs/gfs2/locking/dlm/sysfs.c
@@ -190,7 +190,6 @@ static struct kobj_type gdlm_ktype = {
};
static struct kset gdlm_kset = {
- .kobj = {.name = "lock_dlm",},
.ktype = &gdlm_ktype,
};
@@ -224,6 +223,7 @@ int gdlm_sysfs_init(void)
{
int error;
+ kobject_set_name(&gdlm_kset, "lock_dlm");
kobj_set_kset_s(&gdlm_kset, kernel_subsys);
error = kset_register(&gdlm_kset);
if (error)
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -222,7 +222,6 @@ static struct kobj_type gfs2_ktype = {
};
static struct kset gfs2_kset = {
- .kobj = {.name = "gfs2"},
.ktype = &gfs2_ktype,
};
@@ -553,6 +552,7 @@ int gfs2_sys_init(void)
{
gfs2_sys_margs = NULL;
spin_lock_init(&gfs2_sys_margs_lock);
+ kobject_set_name(&gfs2_kset.kobj, "gfs2");
kobj_set_kset_s(&gfs2_kset, fs_subsys);
return kset_register(&gfs2_kset);
}
--- a/fs/ocfs2/cluster/masklog.c
+++ b/fs/ocfs2/cluster/masklog.c
@@ -143,7 +143,7 @@ static struct kobj_type mlog_ktype = {
};
static struct kset mlog_kset = {
- .kobj = {.name = "logmask", .ktype = &mlog_ktype},
+ .kobj = {.ktype = &mlog_ktype},
};
int mlog_sys_init(struct kset *o2cb_subsys)
@@ -156,6 +156,7 @@ int mlog_sys_init(struct kset *o2cb_subs
}
mlog_attr_ptrs[i] = NULL;
+ kobject_set_name(&mlog_kset.kobj, "logmask");
kobj_set_kset_s(&mlog_kset, *o2cb_subsys);
return kset_register(&mlog_kset);
}
struct cdev does not need the kobject name to be set, as it is never
used. This patch fixes up the few places it is set.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
block/bsg.c | 5 +----
drivers/char/raw.c | 5 +----
drivers/media/dvb/dvb-core/dvbdev.c | 5 +----
drivers/usb/core/devio.c | 6 ++----
4 files changed, 5 insertions(+), 16 deletions(-)
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -1010,10 +1010,7 @@ unlock:
}
EXPORT_SYMBOL_GPL(bsg_register_queue);
-static struct cdev bsg_cdev = {
- .kobj = {.name = "bsg", },
- .owner = THIS_MODULE,
-};
+static struct cdev bsg_cdev;
static int __init bsg_init(void)
{
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -255,10 +255,7 @@ static const struct file_operations raw_
.owner = THIS_MODULE,
};
-static struct cdev raw_cdev = {
- .kobj = {.name = "raw", },
- .owner = THIS_MODULE,
-};
+static struct cdev raw_cdev;
static int __init raw_init(void)
{
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -109,10 +109,7 @@ static struct file_operations dvb_device
.open = dvb_device_open,
};
-static struct cdev dvb_device_cdev = {
- .kobj = {.name = "dvb", },
- .owner = THIS_MODULE,
-};
+static struct cdev dvb_device_cdev;
int dvb_generic_open(struct inode *inode, struct file *file)
{
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1576,6 +1576,7 @@ static unsigned int usbdev_poll(struct f
}
const struct file_operations usbdev_file_operations = {
+ .owner = THIS_MODULE,
.llseek = usbdev_lseek,
.read = usbdev_read,
.poll = usbdev_poll,
@@ -1625,10 +1626,7 @@ static struct notifier_block usbdev_nb =
};
#endif
-static struct cdev usb_device_cdev = {
- .kobj = {.name = "usb_device", },
- .owner = THIS_MODULE,
-};
+static struct cdev usb_device_cdev;
int __init usb_devio_init(void)
{
<resend with proper subject:>
struct cdev does not need the kobject name to be set, as it is never
used. This patch fixes up the few places it is set.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
block/bsg.c | 5 +----
drivers/char/raw.c | 5 +----
drivers/media/dvb/dvb-core/dvbdev.c | 5 +----
drivers/usb/core/devio.c | 6 ++----
4 files changed, 5 insertions(+), 16 deletions(-)
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -1010,10 +1010,7 @@ unlock:
}
EXPORT_SYMBOL_GPL(bsg_register_queue);
-static struct cdev bsg_cdev = {
- .kobj = {.name = "bsg", },
- .owner = THIS_MODULE,
-};
+static struct cdev bsg_cdev;
static int __init bsg_init(void)
{
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -255,10 +255,7 @@ static const struct file_operations raw_
.owner = THIS_MODULE,
};
-static struct cdev raw_cdev = {
- .kobj = {.name = "raw", },
- .owner = THIS_MODULE,
-};
+static struct cdev raw_cdev;
static int __init raw_init(void)
{
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -109,10 +109,7 @@ static struct file_operations dvb_device
.open = dvb_device_open,
};
-static struct cdev dvb_device_cdev = {
- .kobj = {.name = "dvb", },
- .owner = THIS_MODULE,
-};
+static struct cdev dvb_device_cdev;
int dvb_generic_open(struct inode *inode, struct file *file)
{
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1576,6 +1576,7 @@ static unsigned int usbdev_poll(struct f
}
const struct file_operations usbdev_file_operations = {
+ .owner = THIS_MODULE,
.llseek = usbdev_lseek,
.read = usbdev_read,
.poll = usbdev_poll,
@@ -1625,10 +1626,7 @@ static struct notifier_block usbdev_nb =
};
#endif
-static struct cdev usb_device_cdev = {
- .kobj = {.name = "usb_device", },
- .owner = THIS_MODULE,
-};
+static struct cdev usb_device_cdev;
int __init usb_devio_init(void)
{
A number of different drivers incorrect access the kobject name field
directly. This is not correct as the name might not be in the array.
Use the proper accessor function instead.
---
block/elevator.c | 2 +-
block/ll_rw_blk.c | 2 +-
drivers/acpi/bus.c | 2 +-
drivers/cpufreq/cpufreq.c | 2 +-
drivers/md/md.c | 3 +--
fs/partitions/check.c | 12 +++++++-----
net/bridge/br_sysfs_br.c | 2 +-
7 files changed, 13 insertions(+), 12 deletions(-)
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -186,7 +186,7 @@ static elevator_t *elevator_alloc(struct
eq->ops = &e->ops;
eq->elevator_type = e;
kobject_init(&eq->kobj);
- snprintf(eq->kobj.name, KOBJ_NAME_LEN, "%s", "iosched");
+ kobject_set_name(&eq->kobj, "%s", "iosched");
eq->kobj.ktype = &elv_ktype;
mutex_init(&eq->sysfs_lock);
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -1841,7 +1841,7 @@ struct request_queue *blk_alloc_queue_no
init_timer(&q->unplug_timer);
- snprintf(q->kobj.name, KOBJ_NAME_LEN, "%s", "queue");
+ kobject_set_name(&q->kobj, "%s", "queue");
q->kobj.ktype = &queue_ktype;
kobject_init(&q->kobj);
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -194,7 +194,7 @@ int acpi_bus_set_power(acpi_handle handl
if (!device->flags.power_manageable) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device `[%s]' is not power manageable\n",
- device->dev.kobj.name));
+ kobject_name(&device->dev.kobj)));
return -ENODEV;
}
/*
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -828,7 +828,7 @@ static int cpufreq_add_dev (struct sys_d
/* prepare interface data */
policy->kobj.parent = &sys_dev->kobj;
policy->kobj.ktype = &ktype_cpufreq;
- strlcpy(policy->kobj.name, "cpufreq", KOBJ_NAME_LEN);
+ kobject_set_name(&policy->kobj, "cpufreq");
ret = kobject_register(&policy->kobj);
if (ret) {
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3085,8 +3085,7 @@ static struct kobject *md_probe(dev_t de
mddev->gendisk = disk;
mutex_unlock(&disks_mutex);
mddev->kobj.parent = &disk->kobj;
- mddev->kobj.k_name = NULL;
- snprintf(mddev->kobj.name, KOBJ_NAME_LEN, "%s", "md");
+ kobject_set_name(&mddev->kobj, "%s", "md");
mddev->kobj.ktype = &md_ktype;
if (kobject_register(&mddev->kobj))
printk(KERN_WARNING "md: cannot register %s/md - name in use\n",
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -381,10 +381,12 @@ void add_partition(struct gendisk *disk,
p->partno = part;
p->policy = disk->policy;
- if (isdigit(disk->kobj.name[strlen(disk->kobj.name)-1]))
- snprintf(p->kobj.name,KOBJ_NAME_LEN,"%sp%d",disk->kobj.name,part);
+ if (isdigit(disk->kobj.k_name[strlen(disk->kobj.k_name)-1]))
+ kobject_set_name(&p->kobj, "%sp%d",
+ kobject_name(&disk->kobj), part);
else
- snprintf(p->kobj.name,KOBJ_NAME_LEN,"%s%d",disk->kobj.name,part);
+ kobject_set_name(&p->kobj, "%s%d",
+ kobject_name(&disk->kobj),part);
p->kobj.parent = &disk->kobj;
p->kobj.ktype = &ktype_part;
kobject_init(&p->kobj);
@@ -477,9 +479,9 @@ void register_disk(struct gendisk *disk)
struct hd_struct *p;
int err;
- strlcpy(disk->kobj.name,disk->disk_name,KOBJ_NAME_LEN);
+ kobject_set_name(&disk->kobj, "%s", disk->disk_name);
/* ewww... some of these buggers have / in name... */
- s = strchr(disk->kobj.name, '/');
+ s = strchr(disk->kobj.k_name, '/');
if (s)
*s = '!';
if ((err = kobject_add(&disk->kobj)))
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -435,7 +435,7 @@ int br_sysfs_addbr(struct net_device *de
err = kobject_register(&br->ifobj);
if (err) {
pr_info("%s: can't add kobject (directory) %s/%s\n",
- __FUNCTION__, dev->name, br->ifobj.name);
+ __FUNCTION__, dev->name, kobject_name(&br->ifobj));
goto out3;
}
return 0;
Due to historical reasons, struct kobject contained a static array for
the name, and a dynamic pointer in case the name got bigger than the
array. That's just dumb, as people didn't always know which variable to
reference, even with the accessor for the kobject name.
This patch removes the static array, potentially saving a lot of memory
as the majority of kobjects do not have a very long name.
Thanks to Kay for the idea to do this.
Cc: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
include/linux/kobject.h | 7 ++--
lib/kobject.c | 71 ++++++++++++++++++++++--------------------------
2 files changed, 36 insertions(+), 42 deletions(-)
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -63,7 +63,6 @@ extern const char *kobject_actions[];
struct kobject {
const char * k_name;
- char name[KOBJ_NAME_LEN];
struct kref kref;
struct list_head entry;
struct kobject * parent;
@@ -188,18 +187,18 @@ extern struct kobject * kset_find_obj(st
* Use this when initializing an embedded kset with no other
* fields to initialize.
*/
-#define set_kset_name(str) .kset = { .kobj = { .name = str } }
+#define set_kset_name(str) .kset = { .kobj = { .k_name = str } }
#define decl_subsys(_name,_type,_uevent_ops) \
struct kset _name##_subsys = { \
- .kobj = { .name = __stringify(_name) }, \
+ .kobj = { .k_name = __stringify(_name) }, \
.ktype = _type, \
.uevent_ops =_uevent_ops, \
}
#define decl_subsys_name(_varname,_name,_type,_uevent_ops) \
struct kset _varname##_subsys = { \
- .kobj = { .name = __stringify(_name) }, \
+ .kobj = { .k_name = __stringify(_name) }, \
.ktype = _type, \
.uevent_ops =_uevent_ops, \
}
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -170,7 +170,7 @@ int kobject_shadow_add(struct kobject *k
if (!(kobj = kobject_get(kobj)))
return -ENOENT;
if (!kobj->k_name)
- kobj->k_name = kobj->name;
+ kobject_set_name(kobj, "NO_NAME");
if (!*kobj->k_name) {
pr_debug("kobject attempted to be registered with no name!\n");
WARN_ON(1);
@@ -181,7 +181,7 @@ int kobject_shadow_add(struct kobject *k
pr_debug("kobject %s: registering. parent: %s, set: %s\n",
kobject_name(kobj), parent ? kobject_name(parent) : "<NULL>",
- kobj->kset ? kobj->kset->kobj.name : "<NULL>" );
+ kobj->kset ? kobject_name(&kobj->kset->kobj) : "<NULL>" );
if (kobj->kset) {
spin_lock(&kobj->kset->list_lock);
@@ -255,54 +255,50 @@ int kobject_register(struct kobject * ko
int kobject_set_name(struct kobject * kobj, const char * fmt, ...)
{
int error = 0;
- int limit = KOBJ_NAME_LEN;
+ int limit;
int need;
va_list args;
- char * name;
+ char *name;
- /*
- * First, try the static array
- */
- va_start(args,fmt);
- need = vsnprintf(kobj->name,limit,fmt,args);
+ /* find out how big a buffer we need */
+ name = kmalloc(1024, GFP_KERNEL);
+ if (!name) {
+ error = -ENOMEM;
+ goto done;
+ }
+ va_start(args, fmt);
+ need = vsnprintf(name, 1024, fmt, args);
va_end(args);
- if (need < limit)
- name = kobj->name;
- else {
- /*
- * Need more space? Allocate it and try again
- */
- limit = need + 1;
- name = kmalloc(limit,GFP_KERNEL);
- if (!name) {
- error = -ENOMEM;
- goto Done;
- }
- va_start(args,fmt);
- need = vsnprintf(name,limit,fmt,args);
- va_end(args);
-
- /* Still? Give up. */
- if (need >= limit) {
- kfree(name);
- error = -EFAULT;
- goto Done;
- }
+ kfree(name);
+
+ /* Allocate the new space and copy the string in */
+ limit = need + 1;
+ name = kmalloc(limit, GFP_KERNEL);
+ if (!name) {
+ error = -ENOMEM;
+ goto done;
+ }
+ va_start(args, fmt);
+ need = vsnprintf(name, limit, fmt, args);
+ va_end(args);
+
+ /* something wrong with the string we copied? */
+ if (need >= limit) {
+ kfree(name);
+ error = -EFAULT;
+ goto done;
}
/* Free the old name, if necessary. */
- if (kobj->k_name && kobj->k_name != kobj->name)
- kfree(kobj->k_name);
+ kfree(kobj->k_name);
/* Now, set the new name */
kobj->k_name = name;
- Done:
+done:
return error;
}
-
EXPORT_SYMBOL(kobject_set_name);
-
/**
* kobject_rename - change the name of an object
* @kobj: object in question.
@@ -479,8 +475,7 @@ void kobject_cleanup(struct kobject * ko
struct kobject * parent = kobj->parent;
pr_debug("kobject %s: cleaning up\n",kobject_name(kobj));
- if (kobj->k_name != kobj->name)
- kfree(kobj->k_name);
+ kfree(kobj->k_name);
kobj->k_name = NULL;
if (t && t->release)
t->release(kobj);
On Thu, 13 Sep 2007 16:38:50 -0700,
Greg KH <[email protected]> wrote:
> get_bus() should not be globally visable as it is not used by anything
> other than drivers/base/bus.c. This patch removes the visability of it,
Makes sense.
> and renames it to match all of the other *_get() functions in the
> kernel.
What about get_device() and get_driver()? (Not that I want to change
those, but I think the driver core should stick to one naming
convention.)
On Thu, 13 Sep 2007 16:39:47 -0700,
Greg KH <[email protected]> wrote:
> put_bus() should not be globally visable as it is not used by anything
> other than drivers/base/bus.c. This patch removes the visability of it,
> and renames it to match all of the other *_put() functions in the
> kernel.
Same comment as for get_bus(): There's put_device() and put_driver().
On Thu, 13 Sep 2007 16:37:51 -0700,
Greg KH <[email protected]> wrote:
> Kay pointed out to me the other day that we are dragging around 20 bytes
> in every struct kobject and in every struct device to contain a name
> string that can be dynamically allocated instead. For small device
> names (the majority), this savings can add up, especially with a lot of
> individual devices.
>
> So, I started out by getting rid of the static array in the kobject
> structure, as we already were dynamically allocating space if it was
> needed.
>
> Of course, this required a number of other minor cleanups through the
> code tree to handle places where we were incorrectly directly accessing
> the kobject name instead of using the "proper" function. I also got
> sidetracked by a few driver core and kobject.h cleanups of macros that
> are no longer needed, or functions that no longer need to be global
> (they were never exported, so we don't have to worry about that mess...)
>
> And I added a change to trigger a warning if we add an attribute to
> sysfs that we have already had created, to help the SCSI developers out
> with their driver model reworks.
>
> So, here's a series of 11 patches that I've added to my tree, and will
> send to Linus when 2.6.24 is opened up.
>
> Any review comments are appreciated. The full diffstat is below showing
> that overall, we did get rid of more code than was added.
Neat. We can get rid of even more stuff if we remove the removed
functions from the Documentation as well.
Signed-off-by: Cornelia Huck <[email protected]>
---
Documentation/kobject.txt | 21 ++-------------------
1 files changed, 2 insertions(+), 19 deletions(-)
--- linux-2.6.orig/Documentation/kobject.txt
+++ linux-2.6/Documentation/kobject.txt
@@ -54,7 +54,6 @@ embedded in larger data structures and r
struct kobject {
const char * k_name;
- char name[KOBJ_NAME_LEN];
struct kref kref;
struct list_head entry;
struct kobject * parent;
@@ -223,18 +222,15 @@ decl_subsys(devices, &ktype_device, &dev
is equivalent to doing:
struct kset devices_subsys = {
- .kobj = {
- .name = "devices",
- },
.ktype = &ktype_devices,
.uevent_ops = &device_uevent_ops,
};
-
+kobject_set_name(&devices_subsys, name);
The objects that are registered with a subsystem that use the
subsystem's default list must have their kset ptr set properly. These
objects may have embedded kobjects or ksets. The
-following helpers make setting the kset easier:
+following helper makes setting the kset easier:
kobj_set_kset_s(obj,subsys)
@@ -242,22 +238,9 @@ kobj_set_kset_s(obj,subsys)
- Assumes that obj->kobj exists, and is a struct kobject.
- Sets the kset of that kobject to the kset <subsys>.
-
-kset_set_kset_s(obj,subsys)
-
-- Assumes that obj->kset exists, and is a struct kset.
-- Sets the kset of the embedded kobject to the kset <subsys>.
-
-subsys_set_kset(obj,subsys)
-
-- Assumes obj->subsys exists, and is a struct subsystem.
-- Sets obj->subsys.kset.kobj.kset to the subsystem's embedded kset.
-
void subsystem_init(struct kset *s);
int subsystem_register(struct kset *s);
void subsystem_unregister(struct kset *s);
-struct kset *subsys_get(struct kset *s);
-void kset_put(struct kset *s);
These are just wrappers around the respective kset_* functions.
On Thu, 13 Sep 2007 16:41:18 -0700,
Greg KH <[email protected]> wrote:
> int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> {
> - if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
> + if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
> + printk(KERN_WARNING, "sysfs: duplicate filename '%s' "
Stray ',' here ------------------->^
> + "can not be created\n", sd->s_name);
> + WARN_ON(1);
> return -EEXIST;
> + }
>
On Fri, Sep 14, 2007 at 01:02:07PM +0200, Cornelia Huck wrote:
> On Thu, 13 Sep 2007 16:38:50 -0700,
> Greg KH <[email protected]> wrote:
>
> > get_bus() should not be globally visable as it is not used by anything
> > other than drivers/base/bus.c. This patch removes the visability of it,
>
> Makes sense.
>
> > and renames it to match all of the other *_get() functions in the
> > kernel.
>
> What about get_device() and get_driver()? (Not that I want to change
> those, but I think the driver core should stick to one naming
> convention.)
Yes, we should stick to one convention, that's why I made these changes.
Also they were self-contained, as you mention, it would be much harder
to change get_device :(
thanks,
greg k-h
Greg KH wrote:
> We want to let people know when we create a duplicate sysfs file, as
> they need to fix up their code.
>
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> fs/sysfs/dir.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -428,8 +428,12 @@ void sysfs_addrm_start(struct sysfs_addr
> */
> int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> {
> - if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
> + if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
> + printk(KERN_WARNING, "sysfs: duplicate filename '%s' "
> + "can not be created\n", sd->s_name);
> + WARN_ON(1);
> return -EEXIST;
> + }
>
> sd->s_parent = sysfs_get(acxt->parent_sd);
>
As a side story:
I've got code which has checks for device_create_file != 0 but keeps
quiet if it got -EEXIST. I rewrote it now so that it does not rely on
the driver core to skip over already existing files. Doing this
revealed a group of old refcounting bugs. :-) Thanks,
--
Stefan Richter
-=====-=-=== =--= -====
http://arcgraph.de/sr/
On Fri, Sep 14, 2007 at 02:06:21PM +0200, Cornelia Huck wrote:
> On Thu, 13 Sep 2007 16:41:18 -0700,
> Greg KH <[email protected]> wrote:
>
>
> > int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> > {
> > - if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
> > + if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
> > + printk(KERN_WARNING, "sysfs: duplicate filename '%s' "
>
> Stray ',' here ------------------->^
Ugh, I suck :(
greg k-h