Re: 2.5.67-mm3: Bad: scheduling while atomic with IEEE1394 then hard freeze ( lockup on CPU0)

[Greg KH]

On Tue, Apr 15, 2003 at 04:34:56PM -0700, Andrew Morton wrote:
> Philippe Gramoullé <[e-mail]> wrote:
> >
> > I'll wait for the fix and will happily try it once it's available.
> 
> Something like this...
> 
> diff -puN lib/kobject.c~kobj_lock-fix lib/kobject.c
> --- 25/lib/kobject.c~kobj_lock-fix	Tue Apr 15 16:31:28 2003
> +++ 25-akpm/lib/kobject.c	Tue Apr 15 16:34:33 2003
> @@ -336,12 +336,14 @@ void kobject_unregister(struct kobject *
>  struct kobject * kobject_get(struct kobject * kobj)
>  {
>  	struct kobject * ret = kobj;
> -	spin_lock(&kobj_lock);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&kobj_lock, flags);
>  	if (kobj && atomic_read(&kobj->refcount) > 0)
>  		atomic_inc(&kobj->refcount);
>  	else
>  		ret = NULL;
> -	spin_unlock(&kobj_lock);
> +	spin_unlock_irqrestore(&kobj_lock, flags);
>  	return ret;
>  }
>  
> @@ -371,10 +373,15 @@ void kobject_cleanup(struct kobject * ko
>  
>  void kobject_put(struct kobject * kobj)
>  {
> -	if (!atomic_dec_and_lock(&kobj->refcount, &kobj_lock))
> -		return;
> -	spin_unlock(&kobj_lock);
> -	kobject_cleanup(kobj);
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	if (atomic_dec_and_lock(&kobj->refcount, &kobj_lock)) {
> +		spin_unlock_irqrestore(&kobj_lock, flags);
> +		kobject_cleanup(kobj);
> +	} else {
> +		local_irq_restore(flags);
> +	}
>  }

CCed Pat, as this is his territory.

Hm yeah, this will fix the problem.  But is there anyway we can do this
without a lock at all?  I think we wouldn't need the lock, if we didn't
test the refcount for > 0, right?  Pat, that just keeps us from getting
a reference count on a kobject that hasn't been initialized, right?
That is a good idea to do, but is it really necessary?

If only atomic_inc_return() was defined for all platforms we might be
able to do the following, dropping the lock entirely:

struct kobject * kobject_get(struct kobject * kobj)
{
	struct kobject * ret = kobj;
	if (kobj)
		if (atomic_inc_return(kobj->refcount) <= 1) {
			atomic_dec(kobj->refcount);
			ret = NULL;
		}
	else
		ret = NULL;
	return ret;
}

void kobject_put(struct kobject * kobj)
{
	if (!atomic_dec(&kobj->refcount))
		return;
	kobject_cleanup(kobj);
}

Or am I missing something?

Anyone know how to whip up a atomic_inc_return() for the platforms
missing it?

thanks,

greg k-h