[pulseaudio-discuss] [PATCH v4 1/2] client audio: Support memfd transport
Arun Raghavan
arun at accosted.net
Mon Apr 25 11:00:22 UTC 2016
On Fri, 2016-04-15 at 23:06 +0200, Ahmed S. Darwish wrote:
> Now that all layers in the stack support memfd blocks, add memfd
> pools support for client context and audio playback data.
>
> Use such memfd pools by default only if the server signals memfd
> support in its connection negotiations.
>
> Also add ability for clients to force-disable memfd transport
> through the `enable-memfd=' client configuration option.
>
> Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
> ---
>
> Notes:
> [v4 changes]
> - updated commit log: memfd used only upon server support
> - protocol version is now bumped in next commit instead
>
> man/pulse-client.conf.5.xml.in | 8 ++++++++
> src/pulse/client-conf.c | 1 +
> src/pulse/client-conf.h | 2 +-
> src/pulse/context.c | 44 +++++++++++++++++++++++++++++++++++++----
> src/pulse/internal.h | 3 +++
> src/pulsecore/mem.h | 9 +++++++++
> src/pulsecore/protocol-native.c | 28 ++++++++++++++++++++++++--
> 7 files changed, 88 insertions(+), 7 deletions(-)
>
> diff --git a/man/pulse-client.conf.5.xml.in b/man/pulse-client.conf.5.xml.in
> index cca2219..b88898c 100644
> --- a/man/pulse-client.conf.5.xml.in
> +++ b/man/pulse-client.conf.5.xml.in
> @@ -102,6 +102,14 @@ License along with PulseAudio; if not, see .
>
>
> enable-shm= Enable data transfer via POSIX
> + or memfd shared memory. Takes a boolean argument, defaults to
> + yes. If set to no, communication with
> + the server will be exclusively done through data-copy over
> + sockets.
> +
> +
> +
> + enable-memfd=. Enable data transfer via memfd
> shared memory. Takes a boolean argument, defaults to
> yes.
>
> diff --git a/src/pulse/client-conf.c b/src/pulse/client-conf.c
> index c23aa6b..a3c9486 100644
> --- a/src/pulse/client-conf.c
> +++ b/src/pulse/client-conf.c
> @@ -141,6 +141,7 @@ void pa_client_conf_load(pa_client_conf *c, bool load_from_x11, bool load_from_e
> { "cookie-file", pa_config_parse_string, &c->cookie_file_from_client_conf, NULL },
> { "disable-shm", pa_config_parse_bool, &c->disable_shm, NULL },
> { "enable-shm", pa_config_parse_not_bool, &c->disable_shm, NULL },
> + { "enable-memfd", pa_config_parse_not_bool, &c->disable_memfd, NULL },
> { "shm-size-bytes", pa_config_parse_size, &c->shm_size, NULL },
> { "auto-connect-localhost", pa_config_parse_bool, &c->auto_connect_localhost, NULL },
> { "auto-connect-display", pa_config_parse_bool, &c->auto_connect_display, NULL },
> diff --git a/src/pulse/client-conf.h b/src/pulse/client-conf.h
> index eac705a..7691ec7 100644
> --- a/src/pulse/client-conf.h
> +++ b/src/pulse/client-conf.h
> @@ -37,7 +37,7 @@ typedef struct pa_client_conf {
> bool cookie_from_x11_valid;
> char *cookie_file_from_application;
> char *cookie_file_from_client_conf;
> - bool autospawn, disable_shm, auto_connect_localhost, auto_connect_display;
> + bool autospawn, disable_shm, disable_memfd, auto_connect_localhost, auto_connect_display;
> size_t shm_size;
> } pa_client_conf;
>
> diff --git a/src/pulse/context.c b/src/pulse/context.c
> index ef39416..69be5f4 100644
> --- a/src/pulse/context.c
> +++ b/src/pulse/context.c
> @@ -173,7 +173,12 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *
> c->srb_template.readfd = -1;
> c->srb_template.writefd = -1;
>
> - type = !c->conf->disable_shm ? PA_MEM_TYPE_SHARED_POSIX : PA_MEM_TYPE_PRIVATE;
> + c->memfd_on_local = (!c->conf->disable_memfd && pa_memfd_is_locally_supported());
> +
> + type = (c->conf->disable_shm) ? PA_MEM_TYPE_PRIVATE :
> + ((!c->memfd_on_local) ?
> + PA_MEM_TYPE_SHARED_POSIX : PA_MEM_TYPE_SHARED_MEMFD);
> +
> if (!(c->mempool = pa_mempool_new(type, c->conf->shm_size, true))) {
>
> if (!c->conf->disable_shm) {
> @@ -482,6 +487,7 @@ static void setup_complete_callback(pa_pdispatch *pd, uint32_t command, uint32_t
> case PA_CONTEXT_AUTHORIZING: {
> pa_tagstruct *reply;
> bool shm_on_remote = false;
> + bool memfd_on_remote = false;
>
> if (pa_tagstruct_getu32(t, &c->version) < 0 ||
> !pa_tagstruct_eof(t)) {
> @@ -500,7 +506,15 @@ static void setup_complete_callback(pa_pdispatch *pd, uint32_t command, uint32_t
> not. */
> if (c->version >= 13) {
> shm_on_remote = !!(c->version & 0x80000000U);
> - c->version &= 0x7FFFFFFFU;
> +
> + /* Starting with protocol version 31, the second MSB of the version
> + * tag reflects whether memfd is supported on the other PA end. */
> + if (c->version >= 31)
You should be using (c->version & 0x40000000U) for the comparison to
get the actual protocol version.
Not a comment for this patch, but we should probably introduce some
macros to deal with this flag setting to keep things cleaner.
> + memfd_on_remote = !!(c->version & 0x40000000U);
> +
> + /* Reserve the two most-significant _bytes_ of the version tag
> + * for flags. */
> + c->version &= 0x0000FFFFU;
> }
>
> pa_log_debug("Protocol version: remote %u, local %u", c->version, PA_PROTOCOL_VERSION);
> @@ -526,6 +540,26 @@ static void setup_complete_callback(pa_pdispatch *pd, uint32_t command, uint32_t
> pa_log_debug("Negotiated SHM: %s", pa_yes_no(c->do_shm));
> pa_pstream_enable_shm(c->pstream, c->do_shm);
>
> + c->shm_type = PA_MEM_TYPE_PRIVATE;
> + if (c->do_shm) {
> + if (c->version >= 31 && memfd_on_remote && c->memfd_on_local) {
> + const char *reason;
> +
> + pa_pstream_enable_memfd(c->pstream);
> + if (pa_mempool_is_memfd_backed(c->mempool))
> + if (pa_pstream_register_memfd_mempool(c->pstream, c->mempool, &reason))
> + pa_log("Failed to regester memfd mempool. Reason: %s", reason);
> +
> + /* Even if memfd pool registration fails, the negotiated SHM type
> + * shall remain memfd as both endpoints claim to support it. */
> + c->shm_type = PA_MEM_TYPE_SHARED_MEMFD;
> + } else
> + c->shm_type = PA_MEM_TYPE_SHARED_POSIX;
> + }
> +
> + pa_log_debug("Memfd possible: %s", pa_yes_no(c->memfd_on_local));
> + pa_log_debug("Negotiated SHM type: %s", pa_mem_type_to_string(c->shm_type));
> +
> reply = pa_tagstruct_command(c, PA_COMMAND_SET_CLIENT_NAME, &tag);
>
> if (c->version >= 13) {
> @@ -593,8 +627,10 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
> pa_log_debug("SHM possible: %s", pa_yes_no(c->do_shm));
>
> /* Starting with protocol version 13 we use the MSB of the version
> - * tag for informing the other side if we could do SHM or not */
> - pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0));
> + * tag for informing the other side if we could do SHM or not.
> + * Starting from version 31, second MSB is used to flag memfd support. */
> + pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0) |
> + (c->memfd_on_local ? 0x40000000 : 0));
> pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
>
> #ifdef HAVE_CREDS
> diff --git a/src/pulse/internal.h b/src/pulse/internal.h
> index eefd181..9bbb903 100644
> --- a/src/pulse/internal.h
> +++ b/src/pulse/internal.h
> @@ -88,6 +88,7 @@ struct pa_context {
>
> bool is_local:1;
> bool do_shm:1;
> + bool memfd_on_local:1;
> bool server_specified:1;
> bool no_fail:1;
> bool do_autospawn:1;
> @@ -95,6 +96,8 @@ struct pa_context {
> bool filter_added:1;
> pa_spawn_api spawn_api;
>
> + pa_mem_type_t shm_type;
> +
> pa_strlist *server_list;
>
> char *server;
> diff --git a/src/pulsecore/mem.h b/src/pulsecore/mem.h
> index 11a8086..cba1410 100644
> --- a/src/pulsecore/mem.h
> +++ b/src/pulsecore/mem.h
> @@ -22,6 +22,7 @@
>
> #include
>
> +#include
> #include
>
> typedef enum pa_mem_type {
> @@ -48,4 +49,12 @@ static inline bool pa_mem_type_is_shared(pa_mem_type_t t) {
> return (t == PA_MEM_TYPE_SHARED_POSIX) || (t == PA_MEM_TYPE_SHARED_MEMFD);
> }
>
> +static inline bool pa_memfd_is_locally_supported() {
> +#if defined(HAVE_CREDS) && defined(HAVE_MEMFD)
> + return true;
> +#else
> + return false;
> +#endif
> +}
> +
> #endif
> diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
> index ffa5c4d..3f39431 100644
> --- a/src/pulsecore/protocol-native.c
> +++ b/src/pulsecore/protocol-native.c
> @@ -48,6 +48,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -2676,7 +2677,9 @@ static void command_enable_srbchannel(pa_pdispatch *pd, uint32_t command, uint32
> static void command_auth(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) {
> pa_native_connection *c = PA_NATIVE_CONNECTION(userdata);
> const void*cookie;
> + bool memfd_on_remote = false;
> pa_tagstruct *reply;
> + pa_mem_type_t shm_type;
> bool shm_on_remote = false, do_shm;
>
> pa_native_connection_assert_ref(c);
> @@ -2700,7 +2703,15 @@ static void command_auth(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_ta
> not. */
> if (c->version >= 13) {
> shm_on_remote = !!(c->version & 0x80000000U);
> - c->version &= 0x7FFFFFFFU;
> +
> + /* Starting with protocol version 31, the second MSB of the version
> + * tag reflects whether memfd is supported on the other PA end. */
> + if (c->version >= 31)
> + memfd_on_remote = !!(c->version & 0x40000000U);
Same comment as before here. If you're fine with it, I'll just patch
those two lines locally and push after some sanity testing.
-- Arun
> +
> + /* Reserve the two most-significant _bytes_ of the version tag
> + * for flags. */
> + c->version &= 0x0000FFFFU;
> }
>
> pa_log_debug("Protocol version: remote %u, local %u", c->version, PA_PROTOCOL_VERSION);
> @@ -2787,8 +2798,21 @@ static void command_auth(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_ta
> pa_log_debug("Negotiated SHM: %s", pa_yes_no(do_shm));
> pa_pstream_enable_shm(c->pstream, do_shm);
>
> + shm_type = PA_MEM_TYPE_PRIVATE;
> + if (do_shm) {
> + if (c->version >= 31 && memfd_on_remote && pa_memfd_is_locally_supported()) {
> + pa_pstream_enable_memfd(c->pstream);
> + shm_type = PA_MEM_TYPE_SHARED_MEMFD;
> + } else
> + shm_type = PA_MEM_TYPE_SHARED_POSIX;
> +
> + pa_log_debug("Memfd possible: %s", pa_yes_no(pa_memfd_is_locally_supported()));
> + pa_log_debug("Negotiated SHM type: %s", pa_mem_type_to_string(shm_type));
> + }
> +
> reply = reply_new(tag);
> - pa_tagstruct_putu32(reply, PA_PROTOCOL_VERSION | (do_shm ? 0x80000000 : 0));
> + pa_tagstruct_putu32(reply, PA_PROTOCOL_VERSION | (do_shm ? 0x80000000 : 0) |
> + (pa_memfd_is_locally_supported() ? 0x40000000 : 0));
>
> #ifdef HAVE_CREDS
> {
More information about the pulseaudio-discuss
mailing list