From e34ff4906199d2ebd248ae897ae34f52bea151c9 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 11 Sep 2013 22:29:05 -0400 Subject: sysfs: remove ktype->namespace() invocations in directory code For some unrecognizable reason, namespace information is communicated to sysfs through ktype->namespace() callback when there's *nothing* which needs the use of a callback. The whole sequence of operations is completely synchronous and sysfs operations simply end up calling back into the layer which just invoked it in order to find out the namespace information, which is completely backwards, obfuscates what's going on and unnecessarily tangles two separate layers. This patch doesn't remove ktype->namespace() but shifts its handling to kobject layer. We probably want to get rid of the callback in the long term. This patch adds an explicit param to sysfs_{create|rename|move}_dir() and renames them to sysfs_{create|rename|move}_dir_ns(), respectively. ktype->namespace() invocations are moved to the calling sites of the above functions. A new helper kboject_namespace() is introduced which directly tests kobj_ns_type_operations->type which should give the same result as testing sysfs_fs_type(parent_sd) and returns @kobj's namespace tag as necessary. kobject_namespace() is extern as it will be used from another file in the following patches. This patch should be an equivalent conversion without any functional difference. Signed-off-by: Tejun Heo Cc: Eric W. Biederman Cc: Kay Sievers Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 962175134702..85fb3a161b21 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -18,6 +18,24 @@ #include #include +/** + * kobject_namespace - return @kobj's namespace tag + * @kobj: kobject in question + * + * Returns namespace tag of @kobj if its parent has namespace ops enabled + * and thus @kobj should have a namespace tag associated with it. Returns + * %NULL otherwise. + */ +const void *kobject_namespace(struct kobject *kobj) +{ + const struct kobj_ns_type_operations *ns_ops = kobj_ns_ops(kobj); + + if (!ns_ops || ns_ops->type == KOBJ_NS_TYPE_NONE) + return NULL; + + return kobj->ktype->namespace(kobj); +} + /* * populate_dir - populate directory with attributes. * @kobj: object we're working on. @@ -46,8 +64,9 @@ static int populate_dir(struct kobject *kobj) static int create_dir(struct kobject *kobj) { - int error = 0; - error = sysfs_create_dir(kobj); + int error; + + error = sysfs_create_dir_ns(kobj, kobject_namespace(kobj)); if (!error) { error = populate_dir(kobj); if (error) @@ -428,7 +447,7 @@ int kobject_rename(struct kobject *kobj, const char *new_name) goto out; } - error = sysfs_rename_dir(kobj, new_name); + error = sysfs_rename_dir_ns(kobj, new_name, kobject_namespace(kobj)); if (error) goto out; @@ -472,6 +491,7 @@ int kobject_move(struct kobject *kobj, struct kobject *new_parent) if (kobj->kset) new_parent = kobject_get(&kobj->kset->kobj); } + /* old object path */ devpath = kobject_get_path(kobj, GFP_KERNEL); if (!devpath) { @@ -486,7 +506,7 @@ int kobject_move(struct kobject *kobj, struct kobject *new_parent) sprintf(devpath_string, "DEVPATH_OLD=%s", devpath); envp[0] = devpath_string; envp[1] = NULL; - error = sysfs_move_dir(kobj, new_parent); + error = sysfs_move_dir_ns(kobj, new_parent, kobject_namespace(kobj)); if (error) goto out; old_parent = kobj->parent; -- cgit v1.2.3 From cb26a311578e67769e92a39a0a63476533cb7e12 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 11 Sep 2013 22:29:07 -0400 Subject: sysfs: drop kobj_ns_type handling The way namespace tags are implemented in sysfs is more complicated than necessary. As each tag is a pointer value and required to be non-NULL under a namespace enabled parent, there's no need to record separately what type each tag is or where namespace is enabled. If multiple namespace types are needed, which currently aren't, we can simply compare the tag to a set of allowed tags in the superblock assuming that the tags, being pointers, won't have the same value across multiple types. Also, whether to filter by namespace tag or not can be trivially determined by whether the node has any tagged children or not. This patch rips out kobj_ns_type handling from sysfs. sysfs no longer cares whether specific type of namespace is enabled or not. If a sysfs_dirent has a non-NULL tag, the parent is marked as needing namespace filtering and the value is tested against the allowed set of tags for the superblock (currently only one but increasing this number isn't difficult) and the sysfs_dirent is ignored if it doesn't match. This removes most kobject namespace knowledge from sysfs proper which will enable proper separation and layering of sysfs. The namespace sanity checks in fs/sysfs/dir.c are replaced by the new sanity check in kobject_namespace(). As this is the only place ktype->namespace() is called for sysfs, this doesn't weaken the sanity check significantly. I omitted converting the sanity check in sysfs_do_create_link_sd(). While the check can be shifted to upper layer, mistakes there are well contained and should be easily visible anyway. Signed-off-by: Tejun Heo Cc: Eric W. Biederman Cc: Kay Sievers Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 85fb3a161b21..e769ee3c2fb9 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -29,11 +29,14 @@ const void *kobject_namespace(struct kobject *kobj) { const struct kobj_ns_type_operations *ns_ops = kobj_ns_ops(kobj); + const void *ns; if (!ns_ops || ns_ops->type == KOBJ_NS_TYPE_NONE) return NULL; - return kobj->ktype->namespace(kobj); + ns = kobj->ktype->namespace(kobj); + WARN_ON(!ns); /* @kobj in a namespace is required to have !NULL tag */ + return ns; } /* -- cgit v1.2.3 From eee031649707db3c9920d9498f8d03819b74fc23 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Wed, 11 Sep 2013 13:00:30 -0400 Subject: kobject: introduce kobj_completion A common way to handle kobject lifetimes in embedded in objects with different lifetime rules is to pair the kobject with a struct completion. This introduces a kobj_completion structure that can be used in place of the pairing, along with several convenience functions for initialization, release, and put-and-wait. Signed-off-by: Jeff Mahoney Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index e769ee3c2fb9..a5a9b13b0648 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -13,6 +13,7 @@ */ #include +#include #include #include #include @@ -749,6 +750,55 @@ const struct sysfs_ops kobj_sysfs_ops = { .store = kobj_attr_store, }; +/** + * kobj_completion_init - initialize a kobj_completion object. + * @kc: kobj_completion + * @ktype: type of kobject to initialize + * + * kobj_completion structures can be embedded within structures with different + * lifetime rules. During the release of the enclosing object, we can + * wait on the release of the kobject so that we don't free it while it's + * still busy. + */ +void kobj_completion_init(struct kobj_completion *kc, struct kobj_type *ktype) +{ + init_completion(&kc->kc_unregister); + kobject_init(&kc->kc_kobj, ktype); +} +EXPORT_SYMBOL_GPL(kobj_completion_init); + +/** + * kobj_completion_release - release a kobj_completion object + * @kobj: kobject embedded in kobj_completion + * + * Used with kobject_release to notify waiters that the kobject has been + * released. + */ +void kobj_completion_release(struct kobject *kobj) +{ + struct kobj_completion *kc = kobj_to_kobj_completion(kobj); + complete(&kc->kc_unregister); +} +EXPORT_SYMBOL_GPL(kobj_completion_release); + +/** + * kobj_completion_del_and_wait - release the kobject and wait for it + * @kc: kobj_completion object to release + * + * Delete the kobject from sysfs and drop the reference count. Then wait + * until any other outstanding references are also dropped. This routine + * is only necessary once other references may have been taken on the + * kobject. Typically this happens when the kobject has been published + * to sysfs via kobject_add. + */ +void kobj_completion_del_and_wait(struct kobj_completion *kc) +{ + kobject_del(&kc->kc_kobj); + kobject_put(&kc->kc_kobj); + wait_for_completion(&kc->kc_unregister); +} +EXPORT_SYMBOL_GPL(kobj_completion_del_and_wait); + /** * kset_register - initialize and add a kset. * @k: kset. -- cgit v1.2.3 From 26ea12dec0c84133add937455be76d44fe253d85 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 18 Sep 2013 17:15:36 -0400 Subject: kobject: grab an extra reference on kobject->sd to allow duplicate deletes sysfs currently has a rather weird behavior regarding removals. A directory removal would delete all files directly under it but wouldn't recurse into subdirectories, which, while a bit inconsistent, seems to make sense at the first glance as each directory is supposedly associated with a kobject and each kobject can take care of the directory deletion; however, this doesn't really hold as we have groups which can be directories without a kobject associated with it and require explicit deletions. We're in the process of separating out sysfs from kboject / driver core and want a consistent behavior. A removal should delete either only the specified node or everything under it. I think it is helpful to support recursive atomic removal and later patches will implement it. Such change means that a sysfs_dirent associated with kobject may be deleted before the kobject itself is removed if one of its ancestor gets removed before it. As sysfs_remove_dir() puts the base ref, we may end up with dangling pointer on descendants. This can be solved by holding an extra reference on the sd from kobject. Acquire an extra reference on the associated sysfs_dirent on directory creation and put it after removal. Signed-off-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 151089788c21..2fdf7fa9e9bd 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -76,6 +76,13 @@ static int create_dir(struct kobject *kobj) if (error) sysfs_remove_dir(kobj); } + + /* + * @kobj->sd may be deleted by an ancestor going away. Hold an + * extra reference so that it stays until @kobj is gone. + */ + sysfs_get(kobj->sd); + return error; } @@ -532,10 +539,15 @@ out: */ void kobject_del(struct kobject *kobj) { + struct sysfs_dirent *sd; + if (!kobj) return; + sd = kobj->sd; sysfs_remove_dir(kobj); + sysfs_put(sd); + kobj->state_in_sysfs = 0; kobj_kset_leave(kobj); kobject_put(kobj->parent); -- cgit v1.2.3 From 1461c5be7becc6e65dba5cadb31fb5f4339609f5 Mon Sep 17 00:00:00 2001 From: Fengguang Wu Date: Wed, 9 Oct 2013 09:26:21 +0800 Subject: kobject: show debug info on delayed kobject release Useful for locating buggy drivers on kernel oops. It may add dozens of new lines to boot dmesg. DEBUG_KOBJECT_RELEASE is hopefully only enabled in debug kernels (like maybe the Fedora rawhide one, or at developers), so being a bit more verbose is likely ok. Signed-off-by: Fengguang Wu Acked-by: Russell King Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 2fdf7fa9e9bd..7a1c203083eb 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -628,7 +628,7 @@ static void kobject_release(struct kref *kref) { struct kobject *kobj = container_of(kref, struct kobject, kref); #ifdef CONFIG_DEBUG_KOBJECT_RELEASE - pr_debug("kobject: '%s' (%p): %s, parent %p (delayed)\n", + pr_info("kobject: '%s' (%p): %s, parent %p (delayed)\n", kobject_name(kobj), kobj, __func__, kobj->parent); INIT_DELAYED_WORK(&kobj->release, kobject_delayed_cleanup); schedule_delayed_work(&kobj->release, HZ); -- cgit v1.2.3