Project

General

Profile

Bug #4123

Locks should not be held across the call to ddi_periodic_delete(9f)

Added by Marcel Telka about 7 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
driver - device drivers
Start date:
2013-09-10
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

There is an obvious deadlock between auimpl_output_callback() and auimpl_engine_close():

> ffffff001e2cbc40::findstack -v
stack pointer for thread ffffff001e2cbc40: ffffff001e2cb980
[ ffffff001e2cb980 resume_from_intr+0xb7() ]
  ffffff001e2cb9b0 swtch+0x82()
  ffffff001e2cba60 turnstile_block+0x262(0, 0, ffffff04ddb47eb0, fffffffffbc07980, 0, 0)
  ffffff001e2cbad0 mutex_vector_enter+0x3c5(ffffff04ddb47eb0)
  ffffff001e2cbb30 auimpl_output_callback+0x3e(ffffff04ddb47940)
  ffffff001e2cbb70 periodic_execute+0xc9(ffffff04f7d01d28)
  ffffff001e2cbba0 ddi_periodic_softintr+0x54(8)
  ffffff001e2cbbf0 av_dispatch_softvect+0x78(8)
  ffffff001e2cbc20 apix_dispatch_softint+0x35(0, 0)
  ffffff001e2059d0 switch_sp_and_call+0x13()
> ffffff04ddb47eb0::mutex
            ADDR  TYPE             HELD MINSPL OLDSPL WAITERS
ffffff04ddb47eb0 adapt ffffff04eeb83400      -      -     yes
> ffffff04eeb83400::findstack
stack pointer for thread ffffff04eeb83400: ffffff001fad7a70
[ ffffff001fad7a70 _resume_from_idle+0xf4() ]
  ffffff001fad7aa0 swtch+0x141()
  ffffff001fad7ae0 cv_wait+0x70()
  ffffff001fad7b20 i_untimeout+0x10b()
  ffffff001fad7b50 ddi_periodic_delete+0x37()
  ffffff001fad7bb0 auimpl_engine_close+0xfc()
  ffffff001fad7bf0 auclnt_close+0xad()
  ffffff001fad7c20 oss_close+0x4f()
  ffffff001fad7c60 audio_close+0x6d()
  ffffff001fad7c90 dev_close+0x31()
  ffffff001fad7ce0 device_close+0xd8()
  ffffff001fad7d70 spec_close+0x17b()
  ffffff001fad7df0 fop_close+0x61()
  ffffff001fad7e30 closef+0x5e()
  ffffff001fad7ea0 closeandsetf+0x398()
  ffffff001fad7ec0 close+0x13()
  ffffff001fad7f10 _sys_sysenter_post_swapgs+0x149()
>

The auimpl_engine_close() is holding e->e_lock during the ddi_periodic_delete() call:

850    mutex_enter(&e->e_lock);
851    sp->s_engine = NULL;
852    list_remove(&e->e_streams, sp);
853    if (list_is_empty(&e->e_streams)) {
854        ENG_STOP(e);
855        ddi_periodic_delete(e->e_periodic);
856        e->e_periodic = 0;
857        e->e_flags &= ENGINE_DRIVER_FLAGS;
858        ENG_CLOSE(e);
859    }
860    mutex_exit(&e->e_lock);

and the periodic auimpl_output_callback() is trying to get the same lock:

462    mutex_enter(&e->e_lock);
463
464    if (e->e_suspended || e->e_failed || !e->e_periodic) {
465        mutex_exit(&e->e_lock);
466        return;
467    }

The exactly same issue is with auimpl_input_callback() - it could race with auimpl_engine_close() too.


Related issues

Related to illumos gate - Bug #3975: ddi_periodic_add(9F) is entirely rubbishResolvedJoshua M. Clulow2013-08-04

Actions
#1

Updated by Marcel Telka about 7 years ago

Happened again. It seems it is worth fixing...

#2

Updated by Marcel Telka over 6 years ago

This deadlock was introduced by #3975.

#3

Updated by Marcel Telka over 6 years ago

The similar deadlock is in iprb(7d) driver between iprb_m_stop() and iprb_periodic() functions.

#4

Updated by Marcel Telka over 6 years ago

  • Status changed from New to In Progress
  • Assignee set to Marcel Telka
#5

Updated by Marcel Telka over 6 years ago

  • Subject changed from Deadlock between auimpl_output_callback() and auimpl_engine_close() to Locks should not be held across the call to ddi_periodic_delete()
#6

Updated by Marcel Telka over 6 years ago

  • Subject changed from Locks should not be held across the call to ddi_periodic_delete() to Locks should not be held across the call to ddi_periodic_delete(9f)
#7

Updated by Marcel Telka over 6 years ago

  • Status changed from In Progress to Pending RTI
#8

Updated by Electric Monk over 6 years ago

git commit 197c9523b8946cf70fab2bc4ee633b18fc5bde68

Author: Marcel Telka <marcel.telka@nexenta.com>

4123 Locks should not be held across the call to ddi_periodic_delete(9f)
Reviewed by: Keith Wesolowski <keith.wesolowski@joyent.com>
Reviewed by: Joshua M. Clulow <josh@sysmgr.org>
Reviewed by: Garrett D'Amore <garrett@damore.org>
Reviewed by: David Pacheco <dap@joyent.com>
Approved by: Dan McDonald <danmcd@omniti.com>

#9

Updated by Marcel Telka over 6 years ago

  • Status changed from Pending RTI to Resolved

Also available in: Atom PDF