[alsa-devel] [PATCH][RFC][alsa-utils 0/9] alsactl: monitor mode friendly for
Hi,
A command of 'alsactl' has 'monitor' mode to print events emitted by observed control nodes. This mode is not friendly for plug-and-play devices.
A bug is reported that the mode consumes 100% of CPU time when detecting disconnection of observed sound card[1]. A process of alsactl doesn't release ALSA control character device at disconnection. This is quite bad in a view of system.
Furthermore, this mode cannot handle control nodes for newly-detected sound card.
This patchset is my attempt to improve the mode to handle disconnection of observed sound card and new detection of sound card.
A rough summary of this patchset: - maintains observed control nodes by link-list - uses epoll(7) instead of poll(2) to avoid following the list every time of poll loop, - control node for disconnected sound card is not observed anymore and dropped from the list. - the list is rescan and observation is updated when detecting new sound card. Linux-specific inotify(7) is used for this purpose.
I'm happy if receiving test reports and indication of new bugs.
[1] [alsa-devel] alsactl monitor spins at 100% cpu after unplugging USB headphones http://mailman.alsa-project.org/pipermail/alsa-devel/2018-September/140580.h...
Takashi Sakamoto (9): alsactl: install signal handler alsactl: split event loop code to a function alsactl: add an iterator of registered instances of sound card alsactl: use epoll(7) instead of poll(2) alsactl: use link list to maintain source of events alsactl: use a list of source for event dispatcher instead of an array of source alsactl: obsolete array for maintenance of handlers alsactl: handle disconnection of sound card alsactl: handle detection of new sound card
alsactl/monitor.c | 403 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 356 insertions(+), 47 deletions(-)
In a mode of 'monitor, event loop runs to dispatch asynchronous event emitted by control node. In this case, installation of signal handler is good for safe cancellation.
This commit install signal handler to allow some operation to clear event loop.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 8351a79..a2abacb 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -20,8 +20,14 @@ #include "aconfig.h" #include "version.h" #include <stdio.h> +#include <stdbool.h> +#include <string.h> +#include <signal.h> #include <alsa/asoundlib.h>
+static int signal_type; +static bool interrupted; + static int open_ctl(const char *name, snd_ctl_t **ctlp) { snd_ctl_t *ctl; @@ -84,6 +90,30 @@ static int print_event(int card, snd_ctl_t *ctl) return 0; }
+static void handle_unix_signal_for_finish(int sig) +{ + signal_type = sig; + interrupted = true; +} + +static int prepare_signal_handler(void) +{ + struct sigaction sa = {0}; + + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + sa.sa_handler = handle_unix_signal_for_finish; + + if (sigaction(SIGABRT, &sa, NULL) < 0) + return -errno; + if (sigaction(SIGINT, &sa, NULL) < 0) + return -errno; + if (sigaction(SIGTERM, &sa, NULL) < 0) + return -errno; + + return 0; +} + #define MAX_CARDS 256
int monitor(const char *name) @@ -93,6 +123,10 @@ int monitor(const char *name) int show_cards; int i, err = 0;
+ err = prepare_signal_handler(); + if (err < 0) + return err; + if (!name) { int card = -1; while (snd_card_next(&card) >= 0 && card >= 0) { @@ -117,14 +151,26 @@ int monitor(const char *name) show_cards = 0; }
+ interrupted = false; for (;ncards > 0;) { struct pollfd fds[ncards];
+ if (interrupted) { + printf("interrupted: %s\n", strsignal(signal_type)); + break; + } + for (i = 0; i < ncards; i++) snd_ctl_poll_descriptors(ctls[i], &fds[i], 1);
err = poll(fds, ncards, -1); if (err <= 0) { + // Catch this case by an above condition statement to + // check value set by signal handler. I note that + // poll(2) returns EINTR even if configured with + // SA_RESTART. + if (errno == EINTR) + continue; err = 0; break; }
In a mode of 'monitor', an event loop runs.
This commit applies a small refactoring to splits the loop into a function for readability.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 78 ++++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index a2abacb..ae1653c 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -90,6 +90,47 @@ static int print_event(int card, snd_ctl_t *ctl) return 0; }
+static int run_dispatcher(snd_ctl_t **ctls, int ncards, int show_cards) +{ + interrupted = false; + int err = 0; + + for (;ncards > 0;) { + struct pollfd fds[ncards]; + int i; + + if (interrupted) { + printf("interrupted: %s\n", strsignal(signal_type)); + break; + } + + for (i = 0; i < ncards; i++) + snd_ctl_poll_descriptors(ctls[i], &fds[i], 1); + + err = poll(fds, ncards, -1); + if (err <= 0) { + // Catch this case by an above condition statement to + // check value set by signal handler. I note that + // poll(2) returns EINTR even if configured with + // SA_RESTART. + if (errno == EINTR) + continue; + err = 0; + break; + } + + for (i = 0; i < ncards; i++) { + unsigned short revents; + snd_ctl_poll_descriptors_revents(ctls[i], &fds[i], 1, + &revents); + if (revents & POLLIN) + print_event(show_cards ? i : -1, ctls[i]); + } + } + + return err; +} + static void handle_unix_signal_for_finish(int sig) { signal_type = sig; @@ -121,7 +162,8 @@ int monitor(const char *name) snd_ctl_t *ctls[MAX_CARDS]; int ncards = 0; int show_cards; - int i, err = 0; + int i; + int err = 0;
err = prepare_signal_handler(); if (err < 0) @@ -151,39 +193,7 @@ int monitor(const char *name) show_cards = 0; }
- interrupted = false; - for (;ncards > 0;) { - struct pollfd fds[ncards]; - - if (interrupted) { - printf("interrupted: %s\n", strsignal(signal_type)); - break; - } - - for (i = 0; i < ncards; i++) - snd_ctl_poll_descriptors(ctls[i], &fds[i], 1); - - err = poll(fds, ncards, -1); - if (err <= 0) { - // Catch this case by an above condition statement to - // check value set by signal handler. I note that - // poll(2) returns EINTR even if configured with - // SA_RESTART. - if (errno == EINTR) - continue; - err = 0; - break; - } - - for (i = 0; i < ncards; i++) { - unsigned short revents; - snd_ctl_poll_descriptors_revents(ctls[i], &fds[i], 1, - &revents); - if (revents & POLLIN) - print_event(show_cards ? i : -1, ctls[i]); - } - } - + err = run_dispatcher(ctls, ncards, show_cards); error: for (i = 0; i < ncards; i++) snd_ctl_close(ctls[i]);
In a mode of 'monitor', when given no argument, all of available control node is observed for their events. At present, discovering the nodes is done according to sound card number, instead of listing nodes in configuration space of alsa-lib.
This commit adds a structure to discover sound cards with a simple interface.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 48 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index ae1653c..79e8fd9 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -28,6 +28,36 @@ static int signal_type; static bool interrupted;
+#define MAX_CARDS 256 + +struct snd_card_iterator { + int card; + char name[16]; +}; + +void snd_card_iterator_init(struct snd_card_iterator *iter) +{ + iter->card = -1; + memset(iter->name, 0, sizeof(iter->name)); +} + +static const char *snd_card_iterator_next(struct snd_card_iterator *iter) +{ + if (snd_card_next(&iter->card) < 0) + return NULL; + if (iter->card < 0) + return NULL; + if (iter->card >= MAX_CARDS) { + fprintf(stderr, "alsactl: too many cards\n"); + return NULL; + } + + + snprintf(iter->name, sizeof(iter->name), "hw:%d", iter->card); + + return (const char *)iter->name; +} + static int open_ctl(const char *name, snd_ctl_t **ctlp) { snd_ctl_t *ctl; @@ -155,8 +185,6 @@ static int prepare_signal_handler(void) return 0; }
-#define MAX_CARDS 256 - int monitor(const char *name) { snd_ctl_t *ctls[MAX_CARDS]; @@ -170,19 +198,15 @@ int monitor(const char *name) return err;
if (!name) { - int card = -1; - while (snd_card_next(&card) >= 0 && card >= 0) { - char cardname[16]; - if (ncards >= MAX_CARDS) { - fprintf(stderr, "alsactl: too many cards\n"); - err = -E2BIG; - goto error; - } - sprintf(cardname, "hw:%d", card); + struct snd_card_iterator iter; + const char *cardname; + + snd_card_iterator_init(&iter); + while ((cardname = snd_card_iterator_next(&iter))) { err = open_ctl(cardname, &ctls[ncards]); if (err < 0) goto error; - ncards++; + ++ncards; } show_cards = 1; } else {
Linux kernel supports unique system call; epoll(7). This allows applications to make associations for descriptor-unique data in a easy way.
This commit uses epoll(7) instead of poll(2) for this point.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 144 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 115 insertions(+), 29 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 79e8fd9..cf5c50a 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -23,6 +23,7 @@ #include <stdbool.h> #include <string.h> #include <signal.h> +#include <sys/epoll.h> #include <alsa/asoundlib.h>
static int signal_type; @@ -120,47 +121,118 @@ static int print_event(int card, snd_ctl_t *ctl) return 0; }
-static int run_dispatcher(snd_ctl_t **ctls, int ncards, int show_cards) +static int operate_dispatcher(int epfd, uint32_t op, struct epoll_event *epev, + snd_ctl_t *ctl) { - interrupted = false; - int err = 0; + struct pollfd *pfds; + int count; + unsigned int pfd_count; + int i; + int err;
- for (;ncards > 0;) { - struct pollfd fds[ncards]; - int i; + count = snd_ctl_poll_descriptors_count(ctl); + if (count < 0) + return count; + if (count == 0) + return -ENXIO; + pfd_count = count; + + pfds = calloc(pfd_count, sizeof(*pfds)); + if (!pfds) + return -ENOMEM; + + count = snd_ctl_poll_descriptors(ctl, pfds, pfd_count); + if (count < 0) { + err = count; + goto end; + } + if (count != pfd_count) { + err = -EIO; + goto end; + }
- if (interrupted) { - printf("interrupted: %s\n", strsignal(signal_type)); + for (i = 0; i < pfd_count; ++i) { + err = epoll_ctl(epfd, op, pfds[i].fd, epev); + if (err < 0) break; - } + } +end: + free(pfds); + return err; +} + +static int prepare_dispatcher(int epfd, snd_ctl_t **ctls, int ncards) +{ + int i; + int err = 0; + + for (i = 0; i < ncards; ++i) { + snd_ctl_t *ctl = ctls[i]; + struct epoll_event ev = { + .events = EPOLLIN, + .data.ptr = (void *)ctl, + }; + err = operate_dispatcher(epfd, EPOLL_CTL_ADD, &ev, ctl); + if (err < 0) + break; + } + + return err; +} + +static int run_dispatcher(int epfd, unsigned int max_ev_count, int show_cards) +{ + struct epoll_event *epev; + int err = 0; + + epev = calloc(max_ev_count, sizeof(*epev)); + if (!epev) + return -ENOMEM;
- for (i = 0; i < ncards; i++) - snd_ctl_poll_descriptors(ctls[i], &fds[i], 1); + while (!interrupted) { + int count; + int i;
- err = poll(fds, ncards, -1); - if (err <= 0) { - // Catch this case by an above condition statement to + count = epoll_wait(epfd, epev, max_ev_count, 200); + if (count < 0) { + err = count; + + // Catch this case by an loop condition statement to // check value set by signal handler. I note that - // poll(2) returns EINTR even if configured with - // SA_RESTART. - if (errno == EINTR) + // epoll_wait(2) returns EINTR even if configured with + // SA_RESTART, as well as catching SIGCONT after + // SIGSTOP/SIGTSTP/SIGTTIN/SIGTTOU. + if (err == EINTR) continue; - err = 0; break; } + if (count == 0) + continue; + + for (i = 0; i < count; ++i) { + struct epoll_event *ev = epev + i; + snd_ctl_t *handle = (snd_ctl_t *)ev->data.ptr;
- for (i = 0; i < ncards; i++) { - unsigned short revents; - snd_ctl_poll_descriptors_revents(ctls[i], &fds[i], 1, - &revents); - if (revents & POLLIN) - print_event(show_cards ? i : -1, ctls[i]); + if (ev->events & EPOLLIN) + print_event(show_cards ? i : -1, handle); } }
+ free(epev); + return err; }
+static void clear_dispatcher(int epfd, snd_ctl_t **ctls, int ncards) +{ + int i; + + for (i = 0; i < ncards; ++i) { + snd_ctl_t *ctl = ctls[i]; + operate_dispatcher(epfd, EPOLL_CTL_DEL, NULL, ctl); + } +} + static void handle_unix_signal_for_finish(int sig) { signal_type = sig; @@ -187,9 +259,10 @@ static int prepare_signal_handler(void)
int monitor(const char *name) { - snd_ctl_t *ctls[MAX_CARDS]; + snd_ctl_t *ctls[MAX_CARDS] = {0}; int ncards = 0; int show_cards; + int epfd; int i; int err = 0;
@@ -197,6 +270,10 @@ int monitor(const char *name) if (err < 0) return err;
+ epfd = epoll_create(1); + if (epfd < 0) + return -errno; + if (!name) { struct snd_card_iterator iter; const char *cardname; @@ -217,9 +294,18 @@ int monitor(const char *name) show_cards = 0; }
- err = run_dispatcher(ctls, ncards, show_cards); - error: - for (i = 0; i < ncards; i++) - snd_ctl_close(ctls[i]); + err = prepare_dispatcher(epfd, ctls, ncards); + if (err >= 0) + err = run_dispatcher(epfd, ncards, show_cards); + clear_dispatcher(epfd, ctls, ncards); + +error: + for (i = 0; i < ncards; i++) { + if (ctls[i]) + snd_ctl_close(ctls[i]); + } + + close(epfd); + return err; }
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index cf5c50a..8b16307 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -29,6 +29,16 @@ static int signal_type; static bool interrupted;
+struct src_entry { + snd_ctl_t *handle; + char *name; + unsigned int pfd_count; + struct { + struct src_entry *prev; + struct src_entry *next; + } list; +}; + #define MAX_CARDS 256
struct snd_card_iterator { @@ -59,6 +69,72 @@ static const char *snd_card_iterator_next(struct snd_card_iterator *iter) return (const char *)iter->name; }
+static inline void remove_source_entry(struct src_entry *src) +{ + struct src_entry *prev = src->list.prev; + struct src_entry *next = src->list.next; + + if (prev) + prev->list.next = src->list.next; + if (next) + next->list.prev = src->list.prev; + free(src->name); + free(src); +} + +static void clear_source_list(struct src_entry **srcs) +{ + while (*srcs) { + struct src_entry *src = *srcs; + *srcs = src->list.next; + + remove_source_entry(src); + } +} + +static int insert_source_entry(struct src_entry **head, snd_ctl_t *handle, + const char *name) +{ + struct src_entry *src; + int count; + int err; + + src = calloc(1, sizeof(*src)); + if (!src) + return -ENOMEM; + src->handle = handle; + + src->name = strdup(name); + if (!src->name) { + err = -ENOMEM; + goto error; + } + + count = snd_ctl_poll_descriptors_count(handle); + if (count < 0) { + err = count; + goto error; + } + if (count == 0) { + err = -ENXIO; + goto error; + } + src->pfd_count = count; + + if (*head) { + src->list.next = (*head)->list.next; + src->list.prev = *head; + (*head)->list.next = src; + } else { + *head = src; + } + + return 0; +error: + free(src); + return err; +} + static int open_ctl(const char *name, snd_ctl_t **ctlp) { snd_ctl_t *ctl; @@ -259,6 +335,7 @@ static int prepare_signal_handler(void)
int monitor(const char *name) { + struct src_entry *srcs = NULL; snd_ctl_t *ctls[MAX_CARDS] = {0}; int ncards = 0; int show_cards; @@ -281,6 +358,9 @@ int monitor(const char *name) snd_card_iterator_init(&iter); while ((cardname = snd_card_iterator_next(&iter))) { err = open_ctl(cardname, &ctls[ncards]); + if (err < 0) + goto error; + err = insert_source_entry(&srcs, ctls[ncards], cardname); if (err < 0) goto error; ++ncards; @@ -288,6 +368,9 @@ int monitor(const char *name) show_cards = 1; } else { err = open_ctl(name, &ctls[0]); + if (err < 0) + goto error; + err = insert_source_entry(&srcs, ctls[ncards], name); if (err < 0) goto error; ncards++; @@ -300,6 +383,7 @@ int monitor(const char *name) clear_dispatcher(epfd, ctls, ncards);
error: + clear_source_list(&srcs); for (i = 0; i < ncards; i++) { if (ctls[i]) snd_ctl_close(ctls[i]);
Dne 5.10.2018 v 16:47 Takashi Sakamoto napsal(a):
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Thanks for the improved code.
Please, use macros in list.h to manage linked list which are inherited from the kernel here.
Jaroslav
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
alsactl/monitor.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index cf5c50a..8b16307 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -29,6 +29,16 @@ static int signal_type; static bool interrupted;
+struct src_entry {
- snd_ctl_t *handle;
- char *name;
- unsigned int pfd_count;
- struct {
struct src_entry *prev;struct src_entry *next;- } list;
+};
#define MAX_CARDS 256
struct snd_card_iterator { @@ -59,6 +69,72 @@ static const char *snd_card_iterator_next(struct snd_card_iterator *iter) return (const char *)iter->name; }
+static inline void remove_source_entry(struct src_entry *src) +{
- struct src_entry *prev = src->list.prev;
- struct src_entry *next = src->list.next;
- if (prev)
prev->list.next = src->list.next;- if (next)
next->list.prev = src->list.prev;- free(src->name);
- free(src);
+}
+static void clear_source_list(struct src_entry **srcs) +{
- while (*srcs) {
struct src_entry *src = *srcs;*srcs = src->list.next;remove_source_entry(src);- }
+}
+static int insert_source_entry(struct src_entry **head, snd_ctl_t *handle,
const char *name)+{
- struct src_entry *src;
- int count;
- int err;
- src = calloc(1, sizeof(*src));
- if (!src)
return -ENOMEM;- src->handle = handle;
- src->name = strdup(name);
- if (!src->name) {
err = -ENOMEM;goto error;- }
- count = snd_ctl_poll_descriptors_count(handle);
- if (count < 0) {
err = count;goto error;- }
- if (count == 0) {
err = -ENXIO;goto error;- }
- src->pfd_count = count;
- if (*head) {
src->list.next = (*head)->list.next;src->list.prev = *head;(*head)->list.next = src;- } else {
*head = src;- }
- return 0;
+error:
- free(src);
- return err;
+}
static int open_ctl(const char *name, snd_ctl_t **ctlp) { snd_ctl_t *ctl; @@ -259,6 +335,7 @@ static int prepare_signal_handler(void)
int monitor(const char *name) {
- struct src_entry *srcs = NULL; snd_ctl_t *ctls[MAX_CARDS] = {0}; int ncards = 0; int show_cards;
@@ -281,6 +358,9 @@ int monitor(const char *name) snd_card_iterator_init(&iter); while ((cardname = snd_card_iterator_next(&iter))) { err = open_ctl(cardname, &ctls[ncards]);
if (err < 0)goto error;err = insert_source_entry(&srcs, ctls[ncards], cardname); if (err < 0) goto error; ++ncards;@@ -288,6 +368,9 @@ int monitor(const char *name) show_cards = 1; } else { err = open_ctl(name, &ctls[0]);
if (err < 0)goto error;err = insert_source_entry(&srcs, ctls[ncards], name);@@ -300,6 +383,7 @@ int monitor(const char *name) clear_dispatcher(epfd, ctls, ncards);
error:
- clear_source_list(&srcs); for (i = 0; i < ncards; i++) { if (ctls[i]) snd_ctl_close(ctls[i]);
Hi Jaroslav,
On Oct 6 2018 02:14, Jaroslav Kysela wrote:
Dne 5.10.2018 v 16:47 Takashi Sakamoto napsal(a):
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Thanks for the improved code.
Please, use macros in list.h to manage linked list which are inherited from the kernel here.
Let me confirm the macro about which you mention.
This patchset is for alsa-utils. In a package of alsa-utils, there's no such header.
A package of alsa-lib v1.1.6 has 'include/list.h', however this is not installation target.
Would I ask you where the list is available?
Thanks
Takashi Sakamoto
On Sun, 07 Oct 2018 05:04:36 +0200, Takashi Sakamoto wrote:
Hi Jaroslav,
On Oct 6 2018 02:14, Jaroslav Kysela wrote:
Dne 5.10.2018 v 16:47 Takashi Sakamoto napsal(a):
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Thanks for the improved code.
Please, use macros in list.h to manage linked list which are inherited from the kernel here.
Let me confirm the macro about which you mention.
This patchset is for alsa-utils. In a package of alsa-utils, there's no such header.
A package of alsa-lib v1.1.6 has 'include/list.h', however this is not installation target.
Would I ask you where the list is available?
It's fine to put a common list.h into alsa-utils/include.
thanks,
Takashi
Dne 7.10.2018 v 05:04 Takashi Sakamoto napsal(a):
Hi Jaroslav,
On Oct 6 2018 02:14, Jaroslav Kysela wrote:
Dne 5.10.2018 v 16:47 Takashi Sakamoto napsal(a):
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Thanks for the improved code.
Please, use macros in list.h to manage linked list which are inherited from the kernel here.
Let me confirm the macro about which you mention.
This patchset is for alsa-utils. In a package of alsa-utils, there's no such header.
A package of alsa-lib v1.1.6 has 'include/list.h', however this is not installation target.
Would I ask you where the list is available?
[alsa-utils]$ git log alsactl/list.h commit b402cf543aee5f6cd7561d964065a26f07459c8c Author: Jaroslav Kysela perex@perex.cz Date: Thu Jul 31 15:45:08 2008 +0200
Initial 'alsactl init' implementation
See 'man 7 alsactl_init' for more details.
Signed-off-by: Jaroslav Kysela perex@perex.cz
Hi Jaroslav,
On Oct 9 2018 00:14, Jaroslav Kysela wrote:
Dne 7.10.2018 v 05:04 Takashi Sakamoto napsal(a):
Hi Jaroslav,
On Oct 6 2018 02:14, Jaroslav Kysela wrote:
Dne 5.10.2018 v 16:47 Takashi Sakamoto napsal(a):
At present, handlers for control nodes are maintained by one-dimensional array. This is not necessarily useful to maintain handlers with associated information.
This commit adds link-list for the maintenance.
Thanks for the improved code.
Please, use macros in list.h to manage linked list which are inherited from the kernel here.
Let me confirm the macro about which you mention.
This patchset is for alsa-utils. In a package of alsa-utils, there's no such header.
A package of alsa-lib v1.1.6 has 'include/list.h', however this is not installation target.
Would I ask you where the list is available?
[alsa-utils]$ git log alsactl/list.h commit b402cf543aee5f6cd7561d964065a26f07459c8c Author: Jaroslav Kysela perex@perex.cz Date: Thu Jul 31 15:45:08 2008 +0200
Initial 'alsactl init' implementation See 'man 7 alsactl_init' for more details. Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Ah..., exactly. I overlooked this probability. I'll use it for next patchset.
Thanks
Takashi Sakamoto
In a previous commit, handlers of control nodes are maintained by link list.
This commit uses the list to register/unregister event sources to dispatcher.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 76 +++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 8b16307..506f504 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -155,7 +155,7 @@ static int open_ctl(const char *name, snd_ctl_t **ctlp) return 0; }
-static int print_event(int card, snd_ctl_t *ctl) +static int print_event(snd_ctl_t *ctl, const char *name) { snd_ctl_event_t *event; unsigned int mask; @@ -169,9 +169,8 @@ static int print_event(int card, snd_ctl_t *ctl) if (snd_ctl_event_get_type(event) != SND_CTL_EVENT_ELEM) return 0;
- if (card >= 0) - printf("card %d, ", card); - printf("#%d (%i,%i,%i,%s,%i)", + printf("node %s, #%d (%i,%i,%i,%s,%i)", + name, snd_ctl_event_elem_get_numid(event), snd_ctl_event_elem_get_interface(event), snd_ctl_event_elem_get_device(event), @@ -198,36 +197,28 @@ static int print_event(int card, snd_ctl_t *ctl) }
static int operate_dispatcher(int epfd, uint32_t op, struct epoll_event *epev, - snd_ctl_t *ctl) + struct src_entry *src) { struct pollfd *pfds; int count; - unsigned int pfd_count; int i; - int err; - - count = snd_ctl_poll_descriptors_count(ctl); - if (count < 0) - return count; - if (count == 0) - return -ENXIO; - pfd_count = count; + int err = 0;
- pfds = calloc(pfd_count, sizeof(*pfds)); + pfds = calloc(src->pfd_count, sizeof(*pfds)); if (!pfds) return -ENOMEM;
- count = snd_ctl_poll_descriptors(ctl, pfds, pfd_count); + count = snd_ctl_poll_descriptors(src->handle, pfds, src->pfd_count); if (count < 0) { err = count; goto end; } - if (count != pfd_count) { + if (count != src->pfd_count) { err = -EIO; goto end; }
- for (i = 0; i < pfd_count; ++i) { + for (i = 0; i < src->pfd_count; ++i) { err = epoll_ctl(epfd, op, pfds[i].fd, epev); if (err < 0) break; @@ -237,18 +228,17 @@ end: return err; }
-static int prepare_dispatcher(int epfd, snd_ctl_t **ctls, int ncards) +static int prepare_dispatcher(int epfd, struct src_entry *srcs) { - int i; + struct src_entry *src; int err = 0;
- for (i = 0; i < ncards; ++i) { - snd_ctl_t *ctl = ctls[i]; + for (src = srcs; src; src = src->list.next) { struct epoll_event ev = { .events = EPOLLIN, - .data.ptr = (void *)ctl, + .data.ptr = (void *)src, }; - err = operate_dispatcher(epfd, EPOLL_CTL_ADD, &ev, ctl); + err = operate_dispatcher(epfd, EPOLL_CTL_ADD, &ev, src); if (err < 0) break; } @@ -256,11 +246,17 @@ static int prepare_dispatcher(int epfd, snd_ctl_t **ctls, int ncards) return err; }
-static int run_dispatcher(int epfd, unsigned int max_ev_count, int show_cards) +static int run_dispatcher(int epfd, struct src_entry *srcs) { + struct src_entry *src; + unsigned int max_ev_count; struct epoll_event *epev; int err = 0;
+ max_ev_count = 0; + for (src = srcs; src; src = src->list.next) + max_ev_count += src->pfd_count; + epev = calloc(max_ev_count, sizeof(*epev)); if (!epev) return -ENOMEM; @@ -287,10 +283,9 @@ static int run_dispatcher(int epfd, unsigned int max_ev_count, int show_cards)
for (i = 0; i < count; ++i) { struct epoll_event *ev = epev + i; - snd_ctl_t *handle = (snd_ctl_t *)ev->data.ptr; - + struct src_entry *src = (struct src_entry *)ev->data.ptr; if (ev->events & EPOLLIN) - print_event(show_cards ? i : -1, handle); + print_event(src->handle, src->name); } }
@@ -299,14 +294,12 @@ static int run_dispatcher(int epfd, unsigned int max_ev_count, int show_cards) return err; }
-static void clear_dispatcher(int epfd, snd_ctl_t **ctls, int ncards) +static void clear_dispatcher(int epfd, struct src_entry *srcs) { - int i; + struct src_entry *src;
- for (i = 0; i < ncards; ++i) { - snd_ctl_t *ctl = ctls[i]; - operate_dispatcher(epfd, EPOLL_CTL_DEL, NULL, ctl); - } + for (src = srcs; src; src = src->list.next) + operate_dispatcher(epfd, EPOLL_CTL_DEL, NULL, src); }
static void handle_unix_signal_for_finish(int sig) @@ -338,9 +331,7 @@ int monitor(const char *name) struct src_entry *srcs = NULL; snd_ctl_t *ctls[MAX_CARDS] = {0}; int ncards = 0; - int show_cards; int epfd; - int i; int err = 0;
err = prepare_signal_handler(); @@ -365,7 +356,6 @@ int monitor(const char *name) goto error; ++ncards; } - show_cards = 1; } else { err = open_ctl(name, &ctls[0]); if (err < 0) @@ -374,20 +364,14 @@ int monitor(const char *name) if (err < 0) goto error; ncards++; - show_cards = 0; }
- err = prepare_dispatcher(epfd, ctls, ncards); + err = prepare_dispatcher(epfd, srcs); if (err >= 0) - err = run_dispatcher(epfd, ncards, show_cards); - clear_dispatcher(epfd, ctls, ncards); - + err = run_dispatcher(epfd, srcs); + clear_dispatcher(epfd, srcs); error: clear_source_list(&srcs); - for (i = 0; i < ncards; i++) { - if (ctls[i]) - snd_ctl_close(ctls[i]); - }
close(epfd);
In former commits, handlers of control node are maintained by link list, instead of one-dimensional array.
This commit obsoletes the array and split source preparation to a function.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 68 +++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 506f504..807963c 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -39,8 +39,6 @@ struct src_entry { } list; };
-#define MAX_CARDS 256 - struct snd_card_iterator { int card; char name[16]; @@ -58,11 +56,6 @@ static const char *snd_card_iterator_next(struct snd_card_iterator *iter) return NULL; if (iter->card < 0) return NULL; - if (iter->card >= MAX_CARDS) { - fprintf(stderr, "alsactl: too many cards\n"); - return NULL; - } -
snprintf(iter->name, sizeof(iter->name), "hw:%d", iter->card);
@@ -88,6 +81,8 @@ static void clear_source_list(struct src_entry **srcs) struct src_entry *src = *srcs; *srcs = src->list.next;
+ snd_ctl_subscribe_events(src->handle, 0); + snd_ctl_close(src->handle); remove_source_entry(src); } } @@ -131,6 +126,7 @@ static int insert_source_entry(struct src_entry **head, snd_ctl_t *handle,
return 0; error: + free(src->name); free(src); return err; } @@ -155,6 +151,36 @@ static int open_ctl(const char *name, snd_ctl_t **ctlp) return 0; }
+static int prepare_source_entry(struct src_entry **srcs, const char *name) +{ + snd_ctl_t *handle; + int err; + + if (!name) { + struct snd_card_iterator iter; + const char *cardname; + + snd_card_iterator_init(&iter); + while ((cardname = snd_card_iterator_next(&iter))) { + err = open_ctl(cardname, &handle); + if (err < 0) + return err; + err = insert_source_entry(srcs, handle, cardname); + if (err < 0) + return err; + } + } else { + err = open_ctl(name, &handle); + if (err < 0) + return err; + err = insert_source_entry(srcs, handle, name); + if (err < 0) + return err; + } + + return 0; +} + static int print_event(snd_ctl_t *ctl, const char *name) { snd_ctl_event_t *event; @@ -329,8 +355,6 @@ static int prepare_signal_handler(void) int monitor(const char *name) { struct src_entry *srcs = NULL; - snd_ctl_t *ctls[MAX_CARDS] = {0}; - int ncards = 0; int epfd; int err = 0;
@@ -342,29 +366,9 @@ int monitor(const char *name) if (epfd < 0) return -errno;
- if (!name) { - struct snd_card_iterator iter; - const char *cardname; - - snd_card_iterator_init(&iter); - while ((cardname = snd_card_iterator_next(&iter))) { - err = open_ctl(cardname, &ctls[ncards]); - if (err < 0) - goto error; - err = insert_source_entry(&srcs, ctls[ncards], cardname); - if (err < 0) - goto error; - ++ncards; - } - } else { - err = open_ctl(name, &ctls[0]); - if (err < 0) - goto error; - err = insert_source_entry(&srcs, ctls[ncards], name); - if (err < 0) - goto error; - ncards++; - } + err = prepare_source_entry(&srcs, name); + if (err < 0) + goto error;
err = prepare_dispatcher(epfd, srcs); if (err >= 0)
Once sound card becomes disconnection state, corresponding control node becomes to emit error event for listeners. When catching this type of event, event dispatcher should stop observation of the node. However, at present, a mode of monitor can't handle this correctly. As a result, poll(2) is executed quite frequently in loop with no wait. This results 100% consumption of CPU time.
This commit takes the dispatcher to remove the node from observation list when detecting the disconnection state.
Reported-by: Thomas Gläßle thomas@coldfix.de Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 807963c..4eb1ad1 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -312,6 +312,10 @@ static int run_dispatcher(int epfd, struct src_entry *srcs) struct src_entry *src = (struct src_entry *)ev->data.ptr; if (ev->events & EPOLLIN) print_event(src->handle, src->name); + if (ev->events & EPOLLERR) { + operate_dispatcher(epfd, EPOLL_CTL_DEL, NULL, src); + remove_source_entry(src); + } } }
At present, plug-and-play is not supported in a mode of 'monitor', thus new sound card is not handled during runtime. This is not happy.
This commit uses Linux-specific inotify(7) to monitor '/dev/snd' directory. When some files are newly added to the directory, event dispatcher is suspended. Event sources are scanned again and the dispatcher continue to run.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- alsactl/monitor.c | 93 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 13 deletions(-)
diff --git a/alsactl/monitor.c b/alsactl/monitor.c index 4eb1ad1..6b5cfd4 100644 --- a/alsactl/monitor.c +++ b/alsactl/monitor.c @@ -24,6 +24,7 @@ #include <string.h> #include <signal.h> #include <sys/epoll.h> +#include <sys/inotify.h> #include <alsa/asoundlib.h>
static int signal_type; @@ -94,6 +95,12 @@ static int insert_source_entry(struct src_entry **head, snd_ctl_t *handle, int count; int err;
+ // Avoid duplication at retry case. + for (src = *head; src; src = src->list.next) { + if (!strcmp(src->name, name)) + return 0; + } + src = calloc(1, sizeof(*src)); if (!src) return -ENOMEM; @@ -181,6 +188,28 @@ static int prepare_source_entry(struct src_entry **srcs, const char *name) return 0; }
+static int dump_notification(int infd) +{ + char buf[1024]; + ssize_t len; + int err = 0; + + while (1) { + len = read(infd, buf, 1024); + if (len < 0) { + if (errno = EAGAIN) + break; + else + err = errno; + break; + } + if (len == 0) + break; + } + + return err; +} + static int print_event(snd_ctl_t *ctl, const char *name) { snd_ctl_event_t *event; @@ -254,16 +283,20 @@ end: return err; }
-static int prepare_dispatcher(int epfd, struct src_entry *srcs) +static int prepare_dispatcher(int epfd, int infd, struct src_entry *srcs) { + struct epoll_event ev = {0}; struct src_entry *src; int err = 0;
+ ev.events = EPOLLIN; + ev.data.fd = infd; + if (epoll_ctl(epfd, EPOLL_CTL_ADD, infd, &ev) < 0) + return -errno; + for (src = srcs; src; src = src->list.next) { - struct epoll_event ev = { - .events = EPOLLIN, - .data.ptr = (void *)src, - }; + ev.events = EPOLLIN; + ev.data.ptr = (void *)src; err = operate_dispatcher(epfd, EPOLL_CTL_ADD, &ev, src); if (err < 0) break; @@ -272,7 +305,8 @@ static int prepare_dispatcher(int epfd, struct src_entry *srcs) return err; }
-static int run_dispatcher(int epfd, struct src_entry *srcs) +static int run_dispatcher(int epfd, int infd, struct src_entry *srcs, + bool *retry) { struct src_entry *src; unsigned int max_ev_count; @@ -309,7 +343,16 @@ static int run_dispatcher(int epfd, struct src_entry *srcs)
for (i = 0; i < count; ++i) { struct epoll_event *ev = epev + i; - struct src_entry *src = (struct src_entry *)ev->data.ptr; + struct src_entry *src; + + if (ev->data.fd == infd) { + err = dump_notification(infd); + if (err == 0) + *retry = true; + goto end; + } + + src = ev->data.ptr; if (ev->events & EPOLLIN) print_event(src->handle, src->name); if (ev->events & EPOLLERR) { @@ -317,19 +360,22 @@ static int run_dispatcher(int epfd, struct src_entry *srcs) remove_source_entry(src); } } + if (err < 0) + break; } - +end: free(epev); - return err; }
-static void clear_dispatcher(int epfd, struct src_entry *srcs) +static void clear_dispatcher(int epfd, int infd, struct src_entry *srcs) { struct src_entry *src;
for (src = srcs; src; src = src->list.next) operate_dispatcher(epfd, EPOLL_CTL_DEL, NULL, src); + + epoll_ctl(epfd, EPOLL_CTL_DEL, infd, NULL); }
static void handle_unix_signal_for_finish(int sig) @@ -360,6 +406,9 @@ int monitor(const char *name) { struct src_entry *srcs = NULL; int epfd; + int infd; + int wd = 0; + bool retry; int err = 0;
err = prepare_signal_handler(); @@ -370,17 +419,35 @@ int monitor(const char *name) if (epfd < 0) return -errno;
+ infd = inotify_init1(IN_NONBLOCK); + if (infd < 0) { + err = -errno; + goto error; + } + wd = inotify_add_watch(infd, "/dev/snd/", IN_CREATE); + if (wd < 0) { + err = -errno; + goto error; + } +retry: + retry = false; err = prepare_source_entry(&srcs, name); if (err < 0) goto error;
- err = prepare_dispatcher(epfd, srcs); + err = prepare_dispatcher(epfd, infd, srcs); if (err >= 0) - err = run_dispatcher(epfd, srcs); - clear_dispatcher(epfd, srcs); + err = run_dispatcher(epfd, infd, srcs, &retry); + clear_dispatcher(epfd, infd, srcs); + + if (retry) + goto retry; error: + if (wd > 0) + inotify_rm_watch(infd, wd); clear_source_list(&srcs);
+ close(infd); close(epfd);
return err;
Oops. The subject line is truncated, sorry... It should be 'alsactl: monitor mode friendly for plug-and-play devices'.
Thanks
Takashi Sakamoto
participants (3)
-
Jaroslav Kysela -
Takashi Iwai -
Takashi Sakamoto