From c44027c89e19adafccd404bbe6f9686722ff4217 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 11 Oct 2017 06:36:13 +0000 Subject: ALSA: add snd_card_disconnect_sync() In case of user unbind ALSA driver during playing back / capturing, each driver needs to stop and remove it correctly. One note here is that we can't cancel from remove function in such case, because unbind operation doesn't check return value from remove function. So, we *must* stop and remove in this case. For this purpose, we need to sync (= wait) until the all top-level operations are canceled at remove function. For example, snd_card_free() processes the disconnection procedure at first, then waits for the completion. That's how the hot-unplug works safely. It's implemented, at least, in the top-level driver removal. Now for the lower level driver, we need a similar strategy. Notify to the toplevel for hot-unplug (disconnect in ALSA), and sync with the stop operation, then continue the rest of its own remove procedure. This patch adds snd_card_disconnect_sync(), and driver can use it from remove function. Note: the "lower level" driver here refers to a middle layer driver (e.g. ASoC components) that can be unbound freely during operation. Most of legacy ALSA helper drivers don't have such a problem because they can't be unbound. Note#2: snd_card_disconnect_sync() merely calls snd_card_disconnect() and syncs with closing all pending files. It takes only the files opened by user-space into account, and doesn't care about object refcounts. (The latter is handled by snd_card_free() completion call, BTW.) Also, the function doesn't free resources by itself. Tested-by: Kuninori Morimoto Signed-off-by: Takashi Iwai --- include/sound/core.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/sound') diff --git a/include/sound/core.h b/include/sound/core.h index 4104a9d1001f..5f181b875c2f 100644 --- a/include/sound/core.h +++ b/include/sound/core.h @@ -133,6 +133,7 @@ struct snd_card { struct device card_dev; /* cardX object for sysfs */ const struct attribute_group *dev_groups[4]; /* assigned sysfs attr */ bool registered; /* card_dev is registered? */ + wait_queue_head_t remove_sleep; #ifdef CONFIG_PM unsigned int power_state; /* power state */ @@ -240,6 +241,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid, struct snd_card **card_ret); int snd_card_disconnect(struct snd_card *card); +void snd_card_disconnect_sync(struct snd_card *card); int snd_card_free(struct snd_card *card); int snd_card_free_when_closed(struct snd_card *card); void snd_card_set_id(struct snd_card *card, const char *id); -- cgit v1.2.3 From 9780ded39bef5d22a84bdc39112df93f70a58bdd Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 18 Oct 2017 15:51:59 +0200 Subject: ALSA: hda: Avoid racy recreation of widget kobjects The refresh of HD-audio widget sysfs kobjects via snd_hdac_refresh_widget_sysfs() is slightly racy. The driver recreates the whole tree from scratch after deleting the whole. When CONFIG_DEBUG_KOBJECT_RELEASE option is used, kobject release doesn't happen immediately but delayed, while the re-creation of the same named kobject happens soon after invoking kobject_put(). This may end up with the conflicts of duplicated kobjects, as found in the bug report below. In this patch, we take another approach to refresh the tree: instead of recreating the whole tree, just add the new nodes and delete the non-existing nodes. Since the refresh happens only once at initialization, no longer race would happen. Along with the code change, merge snd_hdac_refresh_widget_sysfs() with the existing snd_hdac_refresh_widgets() with an additional bool flag for simplifying the code. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197307 Signed-off-by: Takashi Iwai --- include/sound/hdaudio.h | 3 +-- sound/hda/hdac_device.c | 43 ++++++++++--------------------------------- sound/hda/hdac_sysfs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ sound/hda/local.h | 2 ++ sound/pci/hda/hda_codec.c | 2 +- 5 files changed, 61 insertions(+), 36 deletions(-) (limited to 'include/sound') diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index 96546b30e900..6f1118545bb8 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -111,8 +111,7 @@ void snd_hdac_device_unregister(struct hdac_device *codec); int snd_hdac_device_set_chip_name(struct hdac_device *codec, const char *name); int snd_hdac_codec_modalias(struct hdac_device *hdac, char *buf, size_t size); -int snd_hdac_refresh_widgets(struct hdac_device *codec); -int snd_hdac_refresh_widget_sysfs(struct hdac_device *codec); +int snd_hdac_refresh_widgets(struct hdac_device *codec, bool sysfs); unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid, unsigned int verb, unsigned int parm); diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index 19deb306facb..06f845e293cb 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -87,7 +87,7 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, fg = codec->afg ? codec->afg : codec->mfg; - err = snd_hdac_refresh_widgets(codec); + err = snd_hdac_refresh_widgets(codec, false); if (err < 0) goto error; @@ -388,11 +388,12 @@ static void setup_fg_nodes(struct hdac_device *codec) /** * snd_hdac_refresh_widgets - Reset the widget start/end nodes * @codec: the codec object + * @sysfs: re-initialize sysfs tree, too */ -int snd_hdac_refresh_widgets(struct hdac_device *codec) +int snd_hdac_refresh_widgets(struct hdac_device *codec, bool sysfs) { hda_nid_t start_nid; - int nums; + int nums, err; nums = snd_hdac_get_sub_nodes(codec, codec->afg, &start_nid); if (!start_nid || nums <= 0 || nums >= 0xff) { @@ -401,6 +402,12 @@ int snd_hdac_refresh_widgets(struct hdac_device *codec) return -EINVAL; } + if (sysfs) { + err = hda_widget_sysfs_reinit(codec, start_nid, nums); + if (err < 0) + return err; + } + codec->num_nodes = nums; codec->start_nid = start_nid; codec->end_nid = start_nid + nums; @@ -408,36 +415,6 @@ int snd_hdac_refresh_widgets(struct hdac_device *codec) } EXPORT_SYMBOL_GPL(snd_hdac_refresh_widgets); -/** - * snd_hdac_refresh_widget_sysfs - Reset the codec widgets and reinit the - * codec sysfs - * @codec: the codec object - * - * first we need to remove sysfs, then refresh widgets and lastly - * recreate it - */ -int snd_hdac_refresh_widget_sysfs(struct hdac_device *codec) -{ - int ret; - - if (device_is_registered(&codec->dev)) - hda_widget_sysfs_exit(codec); - ret = snd_hdac_refresh_widgets(codec); - if (ret) { - dev_err(&codec->dev, "failed to refresh widget: %d\n", ret); - return ret; - } - if (device_is_registered(&codec->dev)) { - ret = hda_widget_sysfs_init(codec); - if (ret) { - dev_err(&codec->dev, "failed to init sysfs: %d\n", ret); - return ret; - } - } - return ret; -} -EXPORT_SYMBOL_GPL(snd_hdac_refresh_widget_sysfs); - /* return CONNLIST_LEN parameter of the given widget */ static unsigned int get_num_conns(struct hdac_device *codec, hda_nid_t nid) { diff --git a/sound/hda/hdac_sysfs.c b/sound/hda/hdac_sysfs.c index 42d61bf41969..e123ba6b2e81 100644 --- a/sound/hda/hdac_sysfs.c +++ b/sound/hda/hdac_sysfs.c @@ -414,3 +414,50 @@ void hda_widget_sysfs_exit(struct hdac_device *codec) { widget_tree_free(codec); } + +int hda_widget_sysfs_reinit(struct hdac_device *codec, + hda_nid_t start_nid, int num_nodes) +{ + struct hdac_widget_tree *tree; + hda_nid_t end_nid = start_nid + num_nodes; + hda_nid_t nid; + int i; + + if (!codec->widgets) + return hda_widget_sysfs_init(codec); + + tree = kmemdup(codec->widgets, sizeof(*tree), GFP_KERNEL); + if (!tree) + return -ENOMEM; + + tree->nodes = kcalloc(num_nodes + 1, sizeof(*tree->nodes), GFP_KERNEL); + if (!tree->nodes) { + kfree(tree); + return -ENOMEM; + } + + /* prune non-existing nodes */ + for (i = 0, nid = codec->start_nid; i < codec->num_nodes; i++, nid++) { + if (nid < start_nid || nid >= end_nid) + free_widget_node(codec->widgets->nodes[i], + &widget_node_group); + } + + /* add new nodes */ + for (i = 0, nid = start_nid; i < num_nodes; i++, nid++) { + if (nid < codec->start_nid || nid >= codec->end_nid) + add_widget_node(tree->root, nid, &widget_node_group, + &tree->nodes[i]); + else + tree->nodes[i] = + codec->widgets->nodes[nid - codec->start_nid]; + } + + /* replace with the new tree */ + kfree(codec->widgets->nodes); + kfree(codec->widgets); + codec->widgets = tree; + + kobject_uevent(tree->root, KOBJ_CHANGE); + return 0; +} diff --git a/sound/hda/local.h b/sound/hda/local.h index 0d5bb159d538..c586ea62d49e 100644 --- a/sound/hda/local.h +++ b/sound/hda/local.h @@ -28,6 +28,8 @@ static inline unsigned int get_wcaps_channels(u32 wcaps) extern const struct attribute_group *hdac_dev_attr_groups[]; int hda_widget_sysfs_init(struct hdac_device *codec); +int hda_widget_sysfs_reinit(struct hdac_device *codec, hda_nid_t start_nid, + int num_nodes); void hda_widget_sysfs_exit(struct hdac_device *codec); #endif /* __HDAC_LOCAL_H */ diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 3db26c451837..bb086665c3e0 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -977,7 +977,7 @@ int snd_hda_codec_update_widgets(struct hda_codec *codec) hda_nid_t fg; int err; - err = snd_hdac_refresh_widget_sysfs(&codec->core); + err = snd_hdac_refresh_widgets(&codec->core, true); if (err < 0) return err; -- cgit v1.2.3 From 57e69e2f06e8fe4949c54e438c9faae0731e92f8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 24 Oct 2017 08:35:09 -0700 Subject: ALSA: wavefront: Convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Signed-off-by: Kees Cook Signed-off-by: Takashi Iwai --- include/sound/snd_wavefront.h | 1 + sound/isa/wavefront/wavefront_midi.c | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'include/sound') diff --git a/include/sound/snd_wavefront.h b/include/sound/snd_wavefront.h index cd0bab1ef6f1..fcd770fdcda5 100644 --- a/include/sound/snd_wavefront.h +++ b/include/sound/snd_wavefront.h @@ -28,6 +28,7 @@ struct _snd_wavefront_midi { struct snd_rawmidi_substream *substream_output[2]; struct snd_rawmidi_substream *substream_input[2]; struct timer_list timer; + snd_wavefront_card_t *timer_card; spinlock_t open; spinlock_t virtual; /* protects isvirtual */ }; diff --git a/sound/isa/wavefront/wavefront_midi.c b/sound/isa/wavefront/wavefront_midi.c index 2aa05f3aaa38..556b14738970 100644 --- a/sound/isa/wavefront/wavefront_midi.c +++ b/sound/isa/wavefront/wavefront_midi.c @@ -349,10 +349,10 @@ static void snd_wavefront_midi_input_trigger(struct snd_rawmidi_substream *subst spin_unlock_irqrestore (&midi->virtual, flags); } -static void snd_wavefront_midi_output_timer(unsigned long data) +static void snd_wavefront_midi_output_timer(struct timer_list *t) { - snd_wavefront_card_t *card = (snd_wavefront_card_t *)data; - snd_wavefront_midi_t *midi = &card->wavefront.midi; + snd_wavefront_midi_t *midi = from_timer(midi, t, timer); + snd_wavefront_card_t *card = midi->timer_card; unsigned long flags; spin_lock_irqsave (&midi->virtual, flags); @@ -383,9 +383,9 @@ static void snd_wavefront_midi_output_trigger(struct snd_rawmidi_substream *subs if (up) { if ((midi->mode[mpu] & MPU401_MODE_OUTPUT_TRIGGER) == 0) { if (!midi->istimer) { - setup_timer(&midi->timer, + timer_setup(&midi->timer, snd_wavefront_midi_output_timer, - (unsigned long) substream->rmidi->card->private_data); + 0); mod_timer(&midi->timer, 1 + jiffies); } midi->istimer++; -- cgit v1.2.3