Re: [PATCH RFD] alternative kobject release wait mechanism

[Tejun Heo <htejun@gmail.com>]

Cornelia Huck wrote:
> On Wed, 18 Apr 2007 03:41:10 +0900,
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> Hello, all.
>>
>> Agreed with the problem but I'm not very enthusiastic for adding
>> kobj->owner.  How about the following?  exit() routines will have to
>> do device_unregister_wait() instead of device_unregister().  On return
>> from it, it's guaranteed that all references to it are dropped and
>> ->release is finished.
> 
> Sounds interesting. Kind of like the completion approach, but with the
> dangerous bits outside the module.
> 
>> The caller is responsible for avoiding
>> deadlock, of course.
> 
> I think that wording is a bit too strong. Of course, if the device is
> unregistered in the exit routine, the module must make sure it gave up
> all references it itself obtained. However, it doesn't have any control
> about who obtained a reference during the object's lifetime. If an
> outsider holds on to a reference, we'll lock up until this reference
> has been given up (but I guess this is what we want).

Right, the better wording would be "the caller is responsible for not
causing deadlocks by dropping all references it owns on entry".

>> +void kobject_put_wait(struct kobject * kobj)
>> +{
>> +	struct kobj_wait kwait;
>> +	unsigned long flags;
>> +
>> +	if (!kobj)
>> +		return;
>> +
>> +	BUG_ON(!list_empty(&kobj->entry));
>> +
>> +	init_completion(&kwait.cmpl);
>> +	kwait.kobj = kobj;
>> +
>> +	spin_lock_irqsave(&kobj_wait_lock, flags);
>> +	list_add(&kwait.list, &kobj_wait_list);
>> +	spin_unlock_irqrestore(&kobj_wait_lock, flags);
>> +
>> +	kobject_put(kobj);
>> +
>> +	if (!wait_for_completion_timeout(&kwait.cmpl, 30 * HZ)) {
>> +		printk(KERN_WARNING "kobject_put_wait: kobject %p is still "
>> +		       "alive after 30s, possible reference count bug\n", kobj);
>> +		dump_stack();
>> +		wait_for_completion(&kwait.cmpl);
>> +	}
>> +}
>>
> 
> Couldn't this waiting be made simpler?
> - add completion to kobject in kobject_unregister_wait()
> - call kobject_put(), then wait_for_completion()
> - in kobject_cleanup(), save completion from kobject, call release
> function, then complete() on saved completion

I was trying to avoid adding a completion to all kobjects.

Thanks.

-- 
tejun