Re: 2.6.16-rc1-mm1

[Dave Jones <davej@redhat.com>]

On Wed, Jan 18, 2006 at 03:27:16AM -0800, Andrew Morton wrote:

 > Well yes, that code is kfree()ing a locked mutex.  It's somewhat weird to
 > take a lock on a still-private object but whatever.  The code's legal
 > enough.
 > 
 > 
 > --- devel/drivers/cpufreq/cpufreq.c~cpufreq-mutex-locking-fix	2006-01-18 03:25:33.000000000 -0800
 > +++ devel-akpm/drivers/cpufreq/cpufreq.c	2006-01-18 03:25:55.000000000 -0800
 > @@ -674,6 +674,7 @@ err_out_driver_exit:
 >  		cpufreq_driver->exit(policy);
 >  
 >  err_out:
 > +	mutex_unlock(&policy->lock);
 >  	kfree(policy);
 >  

This looks odd, because we do this..

    mutex_unlock(&policy->lock);

    /* set default policy */

    ret = cpufreq_set_policy(&new_policy);
    if (ret) {
        dprintk("setting policy failed\n");
        goto err_out_unregister;
    }

	...

err_out_unregister:
    spin_lock_irqsave(&cpufreq_driver_lock, flags);
    for_each_cpu_mask(j, policy->cpus)
        cpufreq_cpu_data[j] = NULL;
    spin_unlock_irqrestore(&cpufreq_driver_lock, flags);

    kobject_unregister(&policy->kobj);
    wait_for_completion(&policy->kobj_unregister);

err_out_driver_exit:
    if (cpufreq_driver->exit)
        cpufreq_driver->exit(policy);

err_out:
    kfree(policy);


With the patch above we'll mutex_unlock twice.
Is that allowed ? It sounds wrong to me.

		Dave