Re: [RFC/RFT v2] rfkill: rewrite

Larry Finger
2009-03-31T18:21:08Z

Johannes Berg wrote:
> This patch completely rewrites the rfkill core to address
> the following deficiencies:

-- snip --

> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ wireless-testing/net/rfkill/core.c 2009-03-31 00:04:25.000000000 +0200
> @@ -0,0 +1,752 @@
> +/*

-- snip --

> +int __must_check rfkill_register(struct rfkill *rfkill)
> +{
> + static unsigned long rfkill_no = 0;
> + struct device *dev = &rfkill->dev;
> + int error;
> +
> + BUG_ON(!rfkill);
> +
> + mutex_lock(&rfkill_global_mutex);
> +
> + if (rfkill->registered) {
> + error = -EALREADY;
> + goto unlock;
> + }
> +
> + dev_set_name(dev, "rfkill%lu", rfkill_no);
> + rfkill_no++;
> +
> + if (!(rfkill_states_default_locked & BIT(rfkill->type))) {
> + /* first of its kind */
> + BUILD_BUG_ON(NUM_RFKILL_TYPES >
> + sizeof(rfkill_states_default_locked) * 8);
> + rfkill_states_default_locked |= BIT(rfkill->type);
> + rfkill_global_states[rfkill->type].cur =
> + rfkill_global_states[rfkill->type].def;
> + }
> +
> + /* XXX: schedule work to set default state */
> +
> + list_add_tail(&rfkill->node, &rfkill_list);
> +
> + error = device_add(dev);
> + if (error)
> + goto remove;
> +
> + error = rfkill_led_trigger_register(rfkill);
> + if (error)
> + goto devdel;
> +
> + rfkill->registered = true;
> +
> + if (rfkill->ops->poll_hw_block) {
> + INIT_DELAYED_WORK(&rfkill->poll_work, rfkill_poll);
> + schedule_delayed_work(&rfkill->poll_work,
> + round_jiffies_relative(POLL_INTERVAL));
> + }
> +
> + INIT_WORK(&rfkill->uevent_work, rfkill_uevent_work);
> +
> + return 0;

The locking looks funny here. The normal return leaves the rfkill_global_mutex
locked. Did you mean "goto unlock" instead of "return 0"?

> +
> + devdel:
> + device_del(&rfkill->dev);
> + remove:
> + list_del_init(&rfkill->node);
> + unlock:
> + mutex_unlock(&rfkill_global_mutex);
> + return error;
> +}

I was alerted to this problem when I was unable to shut down or reboot normally.
Those problems are fixed with the patch

Index: wireless-testing/net/rfkill/core.c
===================================================================
--- wireless-testing.orig/net/rfkill/core.c
+++ wireless-testing/net/rfkill/core.c
@@ -676,7 +676,7 @@ int __must_check rfkill_register(struct

INIT_WORK(&rfkill->uevent_work, rfkill_uevent_work);

- return 0;
+ goto unlock;

devdel:
device_del(&rfkill->dev);

Larry