03 Sep, 2014

1 commit

  • This reverts commit 8b37e1bef5a6b60e949e28a4db3006e4b00bd758.

    It's broken as it changes led_blink_set() in a way that it can now sleep
    (while synchronously waiting for workqueue to be cancelled). That's a
    problem, because it's possible that this function gets called from atomic
    context (tpt_trig_timer() takes a readlock and thus disables preemption).

    This has been brought up 3 weeks ago already [1] but no proper fix has
    materialized, and I keep seeing the problem since 3.17-rc1.

    [1] https://lkml.org/lkml/2014/8/16/128

    BUG: sleeping function called from invalid context at kernel/workqueue.c:2650
    in_atomic(): 1, irqs_disabled(): 0, pid: 2335, name: wpa_supplicant
    5 locks held by wpa_supplicant/2335:
    #0: (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x12/0x20
    #1: (&wdev->mtx){+.+.+.}, at: [] cfg80211_mgd_wext_siwessid+0x5c/0x180 [cfg80211]
    #2: (&local->mtx){+.+.+.}, at: [] ieee80211_prep_connection+0x17a/0x9a0 [mac80211]
    #3: (&local->chanctx_mtx){+.+.+.}, at: [] ieee80211_vif_use_channel+0x5d/0x2a0 [mac80211]
    #4: (&trig->leddev_list_lock){.+.+..}, at: [] tpt_trig_timer+0xec/0x170 [mac80211]
    CPU: 0 PID: 2335 Comm: wpa_supplicant Not tainted 3.17.0-rc3 #1
    Hardware name: LENOVO 7470BN2/7470BN2, BIOS 6DET38WW (2.02 ) 12/19/2008
    ffff8800360b5a50 ffff8800751f76d8 ffffffff8159e97f ffff8800360b5a30
    ffff8800751f76e8 ffffffff810739a5 ffff8800751f77b0 ffffffff8106862f
    ffffffff810685d0 0aa2209200000000 ffff880000000004 ffff8800361c59d0
    Call Trace:
    [] dump_stack+0x4d/0x66
    [] __might_sleep+0xe5/0x120
    [] flush_work+0x5f/0x270
    [] ? mod_delayed_work_on+0x80/0x80
    [] ? mark_held_locks+0x6a/0x90
    [] ? __cancel_work_timer+0x6f/0x100
    [] ? trace_hardirqs_on_caller+0xfd/0x1c0
    [] __cancel_work_timer+0x7b/0x100
    [] cancel_delayed_work_sync+0xe/0x10
    [] led_blink_set+0x1b/0x40
    [] tpt_trig_timer+0x110/0x170 [mac80211]
    [] ieee80211_mod_tpt_led_trig+0x9d/0x160 [mac80211]
    [] __ieee80211_recalc_idle+0x98/0x140 [mac80211]
    [] ieee80211_idle_off+0xe/0x10 [mac80211]
    [] ieee80211_add_chanctx+0x3b/0x220 [mac80211]
    [] ieee80211_new_chanctx+0x44/0xf0 [mac80211]
    [] ieee80211_vif_use_channel+0x1fa/0x2a0 [mac80211]
    [] ieee80211_prep_connection+0x188/0x9a0 [mac80211]
    [] ieee80211_mgd_auth+0x256/0x2e0 [mac80211]
    [] ieee80211_auth+0x13/0x20 [mac80211]
    [] cfg80211_mlme_auth+0x106/0x270 [cfg80211]
    [] cfg80211_conn_do_work+0x155/0x3b0 [cfg80211]
    [] cfg80211_connect+0x3f0/0x540 [cfg80211]
    [] cfg80211_mgd_wext_connect+0x158/0x1f0 [cfg80211]
    [] cfg80211_mgd_wext_siwessid+0xde/0x180 [cfg80211]
    [] ? cfg80211_wext_giwessid+0x50/0x50 [cfg80211]
    [] cfg80211_wext_siwessid+0x1d/0x40 [cfg80211]
    [] ioctl_standard_iw_point+0x14c/0x3e0
    [] ? trace_hardirqs_on_caller+0xfd/0x1c0
    [] ioctl_standard_call+0x8a/0xd0
    [] ? ioctl_standard_iw_point+0x3e0/0x3e0
    [] wireless_process_ioctl.constprop.10+0xb6/0x100
    [] wext_handle_ioctl+0x5d/0xb0
    [] dev_ioctl+0x329/0x620
    [] ? trace_hardirqs_on_caller+0xfd/0x1c0
    [] sock_ioctl+0x142/0x2e0
    [] do_vfs_ioctl+0x300/0x520
    [] ? sysret_check+0x1b/0x56
    [] ? trace_hardirqs_on_caller+0xfd/0x1c0
    [] SyS_ioctl+0x81/0xa0
    [] system_call_fastpath+0x1a/0x1f
    wlan0: send auth to 00:0b:6b:3c:8c:e4 (try 1/3)
    wlan0: authenticated
    wlan0: associate with 00:0b:6b:3c:8c:e4 (try 1/3)
    wlan0: RX AssocResp from 00:0b:6b:3c:8c:e4 (capab=0x431 status=0 aid=2)
    wlan0: associated
    IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
    cfg80211: Calling CRDA for country: NA
    wlan0: Limiting TX power to 27 (27 - 0) dBm as advertised by 00:0b:6b:3c:8c:e4

    =================================
    [ INFO: inconsistent lock state ]
    3.17.0-rc3 #1 Not tainted
    ---------------------------------
    inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
    swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
    ((&(&led_cdev->blink_work)->work)){+.?...}, at: [] flush_work+0x0/0x270
    {SOFTIRQ-ON-W} state was registered at:
    [] __lock_acquire+0x30e/0x1a30
    [] lock_acquire+0x91/0x110
    [] flush_work+0x38/0x270
    [] __cancel_work_timer+0x7b/0x100
    [] cancel_delayed_work_sync+0xe/0x10
    [] led_blink_set+0x1b/0x40
    [] tpt_trig_timer+0x110/0x170 [mac80211]
    [] ieee80211_mod_tpt_led_trig+0x9d/0x160 [mac80211]
    [] __ieee80211_recalc_idle+0x98/0x140 [mac80211]
    [] ieee80211_idle_off+0xe/0x10 [mac80211]
    [] ieee80211_add_chanctx+0x3b/0x220 [mac80211]
    [] ieee80211_new_chanctx+0x44/0xf0 [mac80211]
    [] ieee80211_vif_use_channel+0x1fa/0x2a0 [mac80211]
    [] ieee80211_prep_connection+0x188/0x9a0 [mac80211]
    [] ieee80211_mgd_auth+0x256/0x2e0 [mac80211]
    [] ieee80211_auth+0x13/0x20 [mac80211]
    [] cfg80211_mlme_auth+0x106/0x270 [cfg80211]
    [] cfg80211_conn_do_work+0x155/0x3b0 [cfg80211]
    [] cfg80211_connect+0x3f0/0x540 [cfg80211]
    [] cfg80211_mgd_wext_connect+0x158/0x1f0 [cfg80211]
    [] cfg80211_mgd_wext_siwessid+0xde/0x180 [cfg80211]
    [] cfg80211_wext_siwessid+0x1d/0x40 [cfg80211]
    [] ioctl_standard_iw_point+0x14c/0x3e0
    [] ioctl_standard_call+0x8a/0xd0
    [] wireless_process_ioctl.constprop.10+0xb6/0x100
    [] wext_handle_ioctl+0x5d/0xb0
    [] dev_ioctl+0x329/0x620
    [] sock_ioctl+0x142/0x2e0
    [] do_vfs_ioctl+0x300/0x520
    [] SyS_ioctl+0x81/0xa0
    [] system_call_fastpath+0x1a/0x1f
    irq event stamp: 493416
    hardirqs last enabled at (493416): [] __cancel_work_timer+0x6f/0x100
    hardirqs last disabled at (493415): [] try_to_grab_pending+0x1f/0x160
    softirqs last enabled at (493408): [] _local_bh_enable+0x1d/0x50
    softirqs last disabled at (493409): [] irq_exit+0xa5/0xb0

    other info that might help us debug this:
    Possible unsafe locking scenario:

    CPU0
    ----
    lock((&(&led_cdev->blink_work)->work));

    lock((&(&led_cdev->blink_work)->work));

    *** DEADLOCK ***

    2 locks held by swapper/0/0:
    #0: (((&tpt_trig->timer))){+.-...}, at: [] call_timer_fn+0x0/0x180
    #1: (&trig->leddev_list_lock){.+.?..}, at: [] tpt_trig_timer+0xec/0x170 [mac80211]

    stack backtrace:
    CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.17.0-rc3 #1
    Hardware name: LENOVO 7470BN2/7470BN2, BIOS 6DET38WW (2.02 ) 12/19/2008
    ffffffff8246eb30 ffff88007c203b00 ffffffff8159e97f ffffffff81a194c0
    ffff88007c203b50 ffffffff81599c29 0000000000000001 ffffffff00000001
    ffff880000000000 0000000000000006 ffffffff81a194c0 ffffffff81093ad0
    Call Trace:
    [] dump_stack+0x4d/0x66
    [] print_usage_bug+0x1f4/0x205
    [] ? check_usage_backwards+0x140/0x140
    [] mark_lock+0x223/0x2b0
    [] __lock_acquire+0x2b0/0x1a30
    [] lock_acquire+0x91/0x110
    [] ? mod_delayed_work_on+0x80/0x80
    [] ? __ieee80211_get_rx_led_name+0x10/0x10 [mac80211]
    [] flush_work+0x38/0x270
    [] ? mod_delayed_work_on+0x80/0x80
    [] ? mark_held_locks+0x6a/0x90
    [] ? __cancel_work_timer+0x6f/0x100
    [] ? __ieee80211_get_rx_led_name+0x10/0x10 [mac80211]
    [] ? trace_hardirqs_on_caller+0xad/0x1c0
    [] ? __ieee80211_get_rx_led_name+0x10/0x10 [mac80211]
    [] __cancel_work_timer+0x7b/0x100
    [] cancel_delayed_work_sync+0xe/0x10
    [] led_blink_set+0x1b/0x40
    [] tpt_trig_timer+0x110/0x170 [mac80211]
    [] call_timer_fn+0x75/0x180
    [] ? process_timeout+0x10/0x10
    [] ? __ieee80211_get_rx_led_name+0x10/0x10 [mac80211]
    [] run_timer_softirq+0x1fc/0x2f0
    [] __do_softirq+0x115/0x2e0
    [] irq_exit+0xa5/0xb0
    [] do_IRQ+0x53/0xf0
    [] common_interrupt+0x6f/0x6f
    [] ? cpuidle_enter_state+0x6e/0x180
    [] cpuidle_enter+0x12/0x20
    [] cpu_startup_entry+0x330/0x360
    [] rest_init+0xc1/0xd0
    [] ? csum_partial_copy_generic+0x170/0x170
    [] start_kernel+0x44f/0x45a
    [] ? set_init_arg+0x53/0x53
    [] x86_64_start_reservations+0x2a/0x2c
    [] x86_64_start_kernel+0xf1/0xf4

    Cc: Vincent Donnefort
    Cc: Hugh Dickins
    Cc: Tejun Heo
    Signed-off-by: Jiri Kosina
    Signed-off-by: Bryan Wu

    Jiri Kosina
     

04 Jul, 2014

1 commit

  • This patch converts the blink timer from led-core to workqueue which is more
    suitable for this kind of non-priority operations. Moreover, timer may lead to
    errors when a LED setting function use a scheduling function such as pinctrl
    which is using mutex.

    Signed-off-by: Vincent Donnefort
    Signed-off-by: Bryan Wu

    Vincent Donnefort
     

28 Feb, 2014

1 commit


11 Sep, 2012

1 commit


24 Jul, 2012

5 commits

  • drivers/leds/led-core.c:56:6: sparse: symbol 'led_blink_setup' was not declared. Should it be static?
    drivers/leds/led-triggers.c:233:6: sparse: symbol 'led_trigger_blink_setup' was not declared. Should it be static?

    Reported-by: Fengguang Wu
    Signed-off-by: Bryan Wu

    Bryan Wu
     
  • Rename leds external interface led_brightness_set() to led_set_brightness().
    This is the second phase of the change to reduce confusion between the
    leds internal and external interfaces that set brightness. With this change,
    now the external interface is led_set_brightness(). The first phase renamed
    the internal interface led_set_brightness() to __led_set_brightness().
    There are no changes to the interface implementations.

    Signed-off-by: Shuah Khan
    Signed-off-by: Bryan Wu

    Shuah Khan
     
  • Rename leds internal interface led_set_brightness() to __led_set_brightness()
    to reduce confusion between led_set_brightness() and the external interface
    led_brightness_set(). led_brightness_set() cancels the timer and then calls
    led_set_brightness().

    Signed-off-by: Shuah Khan
    Signed-off-by: Bryan Wu

    Shuah Khan
     
  • Move led_stop_software_blink() code into led_brightness_set() to ensure
    software blink timer is stopped and cleared when changing trigger.

    Also use led_set_brightness() instead of calling
    led_cdev->brightness_set() directly to keep led_cdev->brightness
    consistent with current LED status.

    This ensure proper cleaning when changing triggers, as without this fix
    a LED may be turned off while leaving it's led_cdev->brightness = 1,
    leading to an erratic software-blink behaviour.

    The problem was easy to reproduce by changing the trigger from "timer"
    to "oneshot".

    Signed-off-by: Fabio Baltieri
    Signed-off-by: Bryan Wu

    Fabio Baltieri
     
  • Add two new functions, led_blink_set_oneshot and
    led_trigger_blink_oneshot, to be used by triggers for one-shot blink of
    led devices.

    This is implemented extending the existing software-blink code, and uses
    the same timer and handler function.

    The behavior of the code is to do a blink-on, blink-off sequence when
    the function is called, ignoring other calls until the sequence is
    completed so that the leds keep blinking at constant rate if the
    functions are called repeatedly.

    This is meant to be used by drivers which needs to trigger on sporadic
    event, but doesn't have clear busy/idle trigger points.

    After the blink sequence the led remains off. This behavior can be
    inverted setting the "invert" argument, which blink the led off, than on
    and leave the led on after the sequence.

    (bryan.wu@canonical.com: rebase to commit 'leds: don't disable blinking
    when writing the same value to delay_on or delay_off')

    Signed-off-by: Fabio Baltieri
    Acked-by: Shuah Khan
    Signed-off-by: Bryan Wu

    Fabio Baltieri
     

12 Jun, 2012

1 commit

  • Function led_set_software_blink() assumes that blink timer is still running,
    but commit 488bc35bf40df89d37486c1826b178a2fba36ce7 introduced disabling
    of blink timer before each call to led_set_software_blink().

    Correct led_software_blink():
    1) remove protection against reprogramming blink timer to the same values,
    because it only disables blinking now,
    2) remove unnecessary call to led_stop_software_blink().

    Signed-off-by: Rafal Prylowski
    Cc: Richard Purdie
    Signed-off-by: Bryan Wu

    Rafal Prylowski
     

24 Mar, 2012

1 commit