All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: ROJOX <rojoxoficial@gmail.com>
Cc: linux-sound@vger.kernel.org, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC] ALSA: hda: clarify unsol_event locking across driver unbind
Date: Mon, 28 Sep 2026 17:59:03 +0200	[thread overview]
Message-ID: <878q4l4baw.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAP8ibudQFpGuCo+Z97ozKySft0dZ40DWAqf4tUMEe2vrbtu6jA@mail.gmail.com>

On Fri, 25 Sep 2026 04:57:13 +0200,
ROJOX wrote:
> 
> Hi,
> 
> Could you advise on the lifetime and locking contract for
> hdac_driver::unsol_event() relative to codec driver unbind?
> 
> In current torvalds/linux, snd_hdac_bus_process_unsol_events() looks up a
> codec in bus->caddr_tbl under bus->reg_lock, checks codec->registered, and
> then drops reg_lock. It subsequently reads codec->dev.driver and dispatches
> drv->unsol_event(codec, res), without taking a codec device reference or
> device_lock in that interval:
> 
> https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/core/bus.c#L173-L189
> 
> Separately, driver-core unbind acquires the device lock and calls the remove
> path while that lock is held. The HDA reset path can initiate this through
> device_release_driver():
> 
> https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/drivers/base/dd.c#L1315-L1374
> https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/codec.c#L1799-L1810
> 
> This appears to permit the following source-level ordering:
> 
> unsol worker                         unbind task
> ------------                         -----------
> lookup codec under reg_lock
> drop reg_lock
> read codec->dev.driver
> enter unsol_event()
>                                      acquire device_lock
>                                      invoke remove
>                                      tear down private state
> callback continues using private state

Usually the HD-audio controller driver calls snd_hdac_bus_exit() at
its remove callback (or via the destructor invoked from there), and it
does cancel_work_sync() for the unsol event worker.

I thought the removal of codec->dev.driver happened after the
driver_detach(), so at that point, isn't the device object itself
still alive?

Or maybe I haven't followed the flow completely yet.  If the issue is
real, just taking the codec's device reference around the call would
be the easiest solution, I guess.


thanks,

Takashi


> I do not see the worker taking the device lock or another explicit
> binding/private-state lifetime reference before dispatch. If the callback
> uses state destroyed by the remove path, the unlocked dispatch therefore
> appears able to overlap that teardown.
> 
> The codec/device object lifetime, driver binding lifetime, and
> driver-private state lifetime are distinct here: get_device() alone would
> pin the first, but would not by itself prevent unbind or preserve private
> state.
> 
> The legacy HDA wrapper additionally checks shutdown and system-PM state
> before calling the codec callback, but those checks do not appear to drain
> a callback that has already passed them:
> 
> https://github.com/torvalds/linux/blob/165768bb70265b5c38cf0b73fafd75be235f8b14/sound/hda/common/bind.c#L42-L52
> 
> I also found the older unsolicited queue-index synchronization fix,
> c637fa151259c0f74665fde7cba5b7eac1417ae5. That appears to address queue
> consistency rather than this binding/private-state lifetime interval.
> 
> I tested one scratch proof candidate, not a proposed patch. It synchronizes
> address-table lookup/removal, obtains a codec device reference while the
> entry is protected, drops reg_lock, takes device_lock, revalidates the
> current binding and codec registration state, and dispatches the callback
> while that lock is held.
> 
> That closes the ordinary callback-versus-remove schedule in a deterministic
> model, and the modified HDA objects build cleanly in a scratch Ubuntu
> 7.0.0-34.34 source tree. However, hdac_driver::unsol_event() currently does
> not document callback-under-device_lock or reentry restrictions, so I am
> not assuming that this is the correct fix.
> 
> Could you please clarify:
> 
> 1. Is HDA core expected to serialize unsol_event() against unbind by holding
>    the codec device_lock through dispatch, or is there another existing
>    lifetime guarantee intended here?
> 
> 2. If that lock context is acceptable, should the callback contract prohibit
>    recursively taking the same device lock and synchronous same-codec
>    unbind/reset/reprobe? I found no direct same-codec teardown call in the
>    audited in-tree callback bodies, but indirect and out-of-tree behavior is
>    not established.
> 
> 3. Would callback-under-device_lock conflict with expected HDA runtime-PM or
>    system-PM behavior? My source review did not establish a generic contract
>    for this.
> 
> 4. For an event queued before unbind/rebind, is it expected to be deliverable
>    to the newly bound driver, or should it be discarded across the
>    binding/reset boundary?
> 
> This came up while auditing HDA/CS8409 codec lifetime behavior; the question
> is about the generic HDA unsolicited-callback contract.
> 
> No runtime crash has been reproduced. This is based on source/lifetime
> analysis and deterministic concurrency modeling, not a runtime stress test.
> I may be missing an existing lifetime guarantee or invariant, and would
> appreciate correction.
> 
> The source snapshot checked is torvalds/linux master at
> 165768bb70265b5c38cf0b73fafd75be235f8b14. No patch is proposed or attached.
> 
> Thanks,
> Rojox
> iMac19,2 Linux audio project

  reply	other threads:[~2026-09-28 15:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25  2:57 [RFC] ALSA: hda: clarify unsol_event locking across driver unbind ROJOX
2026-09-28 15:59 ` Takashi Iwai [this message]
2026-09-28 16:16   ` ROJOX
2026-09-28 16:27     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878q4l4baw.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=rojoxoficial@gmail.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.