On 9/10/13 2:33 PM, Jeff Mahoney wrote:
> On 9/10/13 2:06 PM, Greg KH wrote:
>> On Tue, Sep 10, 2013 at 01:44:10PM -0400, Jeff Mahoney wrote:
>>> ext4 exports per-filesystem information via sysfs. The lifetime rules
>>> have historically been painful for this but the solution has been to pair
>>> the kobject with a completion and call complete in the kobject's
>>> release function.
>>>
>>> Since this is a pattern I've used in btrfs as well, it makes sense to
>>> turn the pairing into a convenience structure with a standard API.
>>>
>>> Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
>>> ---
>>> include/linux/kobj_completion.h | 18 +++++++++++++++
>>> lib/kobject.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 65 insertions(+)
>>>
>>> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>>> +++ b/include/linux/kobj_completion.h 2013-09-10 12:58:03.530554144 -0400
>>> @@ -0,0 +1,18 @@
>>> +#ifndef _KOBJ_COMPLETION_H_
>>> +#define _KOBJ_COMPLETION_H_
>>> +
>>> +#include <linux/kobject.h>
>>> +#include <linux/completion.h>
>>> +
>>> +struct kobj_completion {
>>> + struct kobject kc_kobj;
>>> + struct completion kc_unregister;
>>> +};
>>> +
>>> +#define kobj_to_kobj_completion(kobj) \
>>> + container_of(kobj, struct kobj_completion, kc_kobj)
>>> +
>>> +void kobj_completion_init(struct kobj_completion *kc, struct kobj_type *ktype);
>>> +void kobj_completion_release(struct kobject *kobj);
>>> +void kobj_completion_del_and_wait(struct kobj_completion *kc);
>>> +#endif /* _KOBJ_COMPLETION_H_ */
>>> --- a/lib/kobject.c 2013-09-10 12:57:54.198666613 -0400
>>> +++ b/lib/kobject.c 2013-09-10 13:16:31.750607946 -0400
>>> @@ -13,6 +13,7 @@
>>> */
>>>
>>> #include <linux/kobject.h>
>>> +#include <linux/kobj_completion.h>
>>> #include <linux/string.h>
>>> #include <linux/export.h>
>>> #include <linux/stat.h>
>>> @@ -711,6 +712,52 @@ const struct sysfs_ops kobj_sysfs_ops =
>>> };
>>>
>>> /**
>>> + * 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 outstanding references are also dropped.
>>> + */
>>> +void kobj_completion_del_and_wait(struct kobj_completion *kc)
>>> +{
>>> + kobject_del(&kc->kc_kobj);
>>> + kobject_put(&kc->kc_kobj);
>>
>> Why the extra kobject_put() call? Who added this extra reference to the
>> object?
>
> There's an assumption that kobject_add will have been called on the
> initialized kobject. If it hasn't been called, the object can just be
> deleted without the completion. It makes the calling code easier to
> read, so would it work for you if I documented that assumption in
> _del_and_wait?
Actually, that's not it. The refcounting works out in my tests.
The kobject's refcount is initialized to 1. kobject_add doesn't
increment it and kobject_del doesn't decrement it. So we need the
kobject_put to trigger the release function.
Or am I missing something here?
-Jeff
--
Jeff Mahoney
SUSE Labs
Attachment:
signature.asc
Description: OpenPGP digital signature
