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

From: Greg KH (greg@kroah.com)
Date: Wed Apr 16 2003 - 00:54:02 EST


On Tue, Apr 15, 2003 at 04:34:56PM -0700, Andrew Morton wrote:
> Philippe Gramoullé <philippe.gramoulle@mmania.com> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Apr 23 2003 - 22:00:17 EST