From patchwork Fri Nov 11 08:00:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juergen Gross X-Patchwork-Id: 9422525 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A5560601C0 for ; Fri, 11 Nov 2016 08:03:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 97A332995A for ; Fri, 11 Nov 2016 08:03:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8C5C829962; Fri, 11 Nov 2016 08:03:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C7BDB29966 for ; Fri, 11 Nov 2016 08:03:14 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c56l5-0007Eg-5m; Fri, 11 Nov 2016 08:00:23 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c56l4-0007D8-Ag for xen-devel@lists.xen.org; Fri, 11 Nov 2016 08:00:22 +0000 Received: from [85.158.137.68] by server-3.bemta-3.messagelabs.com id 81/3A-24885-59A75285; Fri, 11 Nov 2016 08:00:21 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrALMWRWlGSWpSXmKPExsVyuP0Ov+6UKtU Ig1n3TS2WfFzM4sDocXT3b6YAxijWzLyk/IoE1ozZq/axFsyPqljYOIWxgXGFdxcjJ4eEgJHE 24n/mLoYuTiEBBYySrzp2swGkmATUJXYcP0UK4gtIiAtce3zZUYQm1ngFKPEjg1OXYwcHMICI RLdm2VBwixA5acut7CA2LwC9hKdZxezQMyXk7g+czoTiM0JFD8+4QEziC0kYCfRv20T2wRG7g WMDKsY1YtTi8pSi3SN9ZKKMtMzSnITM3N0DQ2M9XJTi4sT01NzEpOK9ZLzczcxAn3LAAQ7GJu /OB1ilORgUhLlZS1UjRDiS8pPqcxILM6ILyrNSS0+xCjDwaEkwatZCZQTLEpNT61Iy8wBBhlM WoKDR0mENwgkzVtckJhbnJkOkTrFqMvxZtfLB0xCLHn5ealS4rwzQIoEQIoySvPgRsAC/hKjr JQwLyPQUUI8BalFuZklqPKvGMU5GJWEeQ1ApvBk5pXAbXoFdAQT0BEz4lRAjihJREhJNTAGRq s7q+h2POI+kpvw9OCae1p5H9ZPSA+Om7TEo++CPKP8+qCc23LuLS/eSCybV7/+OBtPQWQxx/P fjz1YrV6v7siTvhW1r+ioWf2Vv2cCWt8UP5bI7NkZI6Vanslz4rnRbXbtvf/3mWyY/OPGxc4D 7hd2c5+/Ly20S+rpWx4xp7T1Pmpf3lgpsRRnJBpqMRcVJwIA+F5/L3MCAAA= X-Env-Sender: jgross@suse.com X-Msg-Ref: server-12.tower-31.messagelabs.com!1478851220!53590541!1 X-Originating-IP: [195.135.220.15] X-SpamReason: No, hits=0.5 required=7.0 tests=BODY_RANDOM_LONG X-StarScan-Received: X-StarScan-Version: 9.0.16; banners=-,-,- X-VirusChecked: Checked Received: (qmail 7012 invoked from network); 11 Nov 2016 08:00:20 -0000 Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by server-12.tower-31.messagelabs.com with DHE-RSA-CAMELLIA256-SHA encrypted SMTP; 11 Nov 2016 08:00:20 -0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 8492FAD68; Fri, 11 Nov 2016 08:00:20 +0000 (UTC) From: Juergen Gross To: xen-devel@lists.xen.org Date: Fri, 11 Nov 2016 09:00:09 +0100 Message-Id: <1478851210-6251-12-git-send-email-jgross@suse.com> X-Mailer: git-send-email 2.6.6 In-Reply-To: <1478851210-6251-1-git-send-email-jgross@suse.com> References: <1478851210-6251-1-git-send-email-jgross@suse.com> Cc: Juergen Gross , sstabellini@kernel.org, wei.liu2@citrix.com, George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, tim@xen.org, jbeulich@suse.com Subject: [Xen-devel] [PATCH v3 11/12] xenstore: add small default data buffer to internal struct X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Instead of always allocating a data buffer for incoming or outgoing xenstore wire data add a small buffer to the buffered_data structure of xenstored. This has the advantage that especially sending simple response messages like errors or "OK" will no longer need allocating a data buffer. This requires adding a memory context where the allocated buffer was used for that purpose. In order to avoid allocating a new buffered_data structure for each response reuse the structure of the original request. This in turn will avoid any new memory allocations for sending e.g. an ENOMEM response making it possible to send it at all. To do this the allocation of the buffered_data structure for the incoming request must be done when a new request is recognized instead of doing it when accepting a new connect. Signed-off-by: Juergen Gross Acked-by: Wei Liu --- tools/xenstore/xenstored_core.c | 80 +++++++++++++++++++--------------- tools/xenstore/xenstored_core.h | 6 ++- tools/xenstore/xenstored_domain.c | 4 +- tools/xenstore/xenstored_transaction.c | 4 +- tools/xenstore/xenstored_watch.c | 4 +- 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index c7a7c45..f89a636 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -647,17 +647,20 @@ void send_reply(struct connection *conn, enum xsd_sockmsg_type type, return; } - /* Message is a child of the connection context for auto-cleanup. */ - bdata = new_buffer(conn); - bdata->buffer = talloc_array(bdata, char, len); - - /* Echo request header in reply unless this is an async watch event. */ + /* Replies reuse the request buffer, events need a new one. */ if (type != XS_WATCH_EVENT) { - memcpy(&bdata->hdr.msg, &conn->in->hdr.msg, - sizeof(struct xsd_sockmsg)); + bdata = conn->in; + bdata->inhdr = true; + bdata->used = 0; + conn->in = NULL; } else { - memset(&bdata->hdr.msg, 0, sizeof(struct xsd_sockmsg)); + /* Message is a child of the connection for auto-cleanup. */ + bdata = new_buffer(conn); } + if (len <= DEFAULT_BUFFER_SIZE) + bdata->buffer = bdata->default_buffer; + else + bdata->buffer = talloc_array(bdata, char, len); /* Update relevant header fields and fill in the message body. */ bdata->hdr.msg.type = type; @@ -733,7 +736,7 @@ static char *perms_to_strings(const void *ctx, return strings; } -char *canonicalize(struct connection *conn, const char *node) +char *canonicalize(struct connection *conn, const void *ctx, const char *node) { const char *prefix; @@ -741,7 +744,7 @@ char *canonicalize(struct connection *conn, const char *node) return (char *)node; prefix = get_implicit_path(conn); if (prefix) - return talloc_asprintf(node, "%s/%s", prefix, node); + return talloc_asprintf(ctx, "%s/%s", prefix, node); return (char *)node; } @@ -755,7 +758,7 @@ static struct node *get_node_canonicalized(struct connection *conn, if (!canonical_name) canonical_name = &tmp_name; - *canonical_name = canonicalize(conn, name); + *canonical_name = canonicalize(conn, ctx, name); return get_node(conn, ctx, *canonical_name, perm); } @@ -865,17 +868,18 @@ static char *basename(const char *name) return strrchr(name, '/') + 1; } -static struct node *construct_node(struct connection *conn, const char *name) +static struct node *construct_node(struct connection *conn, const void *ctx, + const char *name) { const char *base; unsigned int baselen; struct node *parent, *node; - char *children, *parentname = get_parent(name, name); + char *children, *parentname = get_parent(ctx, name); /* If parent doesn't exist, create it. */ parent = read_node(conn, parentname, parentname); if (!parent) - parent = construct_node(conn, parentname); + parent = construct_node(conn, ctx, parentname); if (!parent) return NULL; @@ -885,14 +889,14 @@ static struct node *construct_node(struct connection *conn, const char *name) /* Add child to parent. */ base = basename(name); baselen = strlen(base) + 1; - children = talloc_array(name, char, parent->childlen + baselen); + children = talloc_array(ctx, char, parent->childlen + baselen); memcpy(children, parent->children, parent->childlen); memcpy(children + parent->childlen, base, baselen); parent->children = children; parent->childlen += baselen; /* Allocate node */ - node = talloc(name, struct node); + node = talloc(ctx, struct node); node->tdb = tdb_context(conn); node->name = talloc_strdup(node, name); @@ -926,13 +930,13 @@ static int destroy_node(void *_node) return 0; } -static struct node *create_node(struct connection *conn, +static struct node *create_node(struct connection *conn, const void *ctx, const char *name, void *data, unsigned int datalen) { struct node *node, *i; - node = construct_node(conn, name); + node = construct_node(conn, ctx, name); if (!node) return NULL; @@ -975,7 +979,8 @@ static int do_write(struct connection *conn, struct buffered_data *in) /* No permissions, invalid input? */ if (errno != ENOENT) return errno; - node = create_node(conn, name, in->buffer + offset, datalen); + node = create_node(conn, in, name, in->buffer + offset, + datalen); if (!node) return errno; } else { @@ -1004,7 +1009,7 @@ static int do_mkdir(struct connection *conn, struct buffered_data *in) /* No permissions? */ if (errno != ENOENT) return errno; - node = create_node(conn, name, NULL, 0); + node = create_node(conn, in, name, NULL, 0); if (!node) return errno; fire_watches(conn, in, name, false); @@ -1075,12 +1080,13 @@ static bool delete_child(struct connection *conn, } -static int _rm(struct connection *conn, struct node *node, const char *name) +static int _rm(struct connection *conn, const void *ctx, struct node *node, + const char *name) { /* Delete from parent first, then if we crash, the worst that can happen is the child will continue to take up space, but will otherwise be unreachable. */ - struct node *parent = read_node(conn, name, get_parent(name, name)); + struct node *parent = read_node(conn, ctx, get_parent(ctx, name)); if (!parent) return EINVAL; @@ -1097,7 +1103,7 @@ static void internal_rm(const char *name) char *tname = talloc_strdup(NULL, name); struct node *node = read_node(NULL, tname, tname); if (node) - _rm(NULL, node, tname); + _rm(NULL, tname, node, tname); talloc_free(node); talloc_free(tname); } @@ -1128,7 +1134,7 @@ static int do_rm(struct connection *conn, struct buffered_data *in) if (streq(name, "/")) return EINVAL; - ret = _rm(conn, node, name); + ret = _rm(conn, in, node, name); if (ret) return ret; @@ -1301,8 +1307,7 @@ static void consider_message(struct connection *conn) process_message(conn, conn->in); - talloc_free(conn->in); - conn->in = new_buffer(conn); + assert(conn->in == NULL); } /* Errors in reading or allocating here mean we get out of sync, so we @@ -1310,7 +1315,15 @@ static void consider_message(struct connection *conn) static void handle_input(struct connection *conn) { int bytes; - struct buffered_data *in = conn->in; + struct buffered_data *in; + + if (!conn->in) { + conn->in = new_buffer(conn); + /* In case of no memory just try it next time again. */ + if (!conn->in) + return; + } + in = conn->in; /* Not finished header yet? */ if (in->inhdr) { @@ -1328,7 +1341,10 @@ static void handle_input(struct connection *conn) goto bad_client; } - in->buffer = talloc_array(in, char, in->hdr.msg.len); + if (in->hdr.msg.len <= DEFAULT_BUFFER_SIZE) + in->buffer = in->default_buffer; + else + in->buffer = talloc_array(in, char, in->hdr.msg.len); if (!in->buffer) goto bad_client; in->used = 0; @@ -1377,12 +1393,6 @@ struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) INIT_LIST_HEAD(&new->watches); INIT_LIST_HEAD(&new->transaction_list); - new->in = new_buffer(new); - if (new->in == NULL) { - talloc_free(new); - return NULL; - } - list_add_tail(&new->list, &connections); talloc_set_destructor(new, destroy_conn); trace_create(new, "connection"); @@ -1522,7 +1532,7 @@ static void setup_structure(void) if (remove_local) { internal_rm("/local"); - create_node(NULL, tlocal, NULL, 0); + create_node(NULL, NULL, tlocal, NULL, 0); check_store(); } diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 3872dab..f6a56f7 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -33,6 +33,9 @@ #include "list.h" #include "tdb.h" +/* DEFAULT_BUFFER_SIZE should be large enough for each errno string. */ +#define DEFAULT_BUFFER_SIZE 16 + struct buffered_data { struct list_head list; @@ -50,6 +53,7 @@ struct buffered_data /* The actual data. */ char *buffer; + char default_buffer[DEFAULT_BUFFER_SIZE]; }; struct connection; @@ -139,7 +143,7 @@ void send_reply(struct connection *conn, enum xsd_sockmsg_type type, void send_ack(struct connection *conn, enum xsd_sockmsg_type type); /* Canonicalize this path if possible. */ -char *canonicalize(struct connection *conn, const char *node); +char *canonicalize(struct connection *conn, const void *ctx, const char *node); /* Get this node, checking we have permissions. */ struct node *get_node(struct connection *conn, diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 2443b08..fefed5e 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -329,9 +329,7 @@ static void domain_conn_reset(struct domain *domain) talloc_free(out); } - talloc_free(conn->in->buffer); - memset(conn->in, 0, sizeof(*conn->in)); - conn->in->inhdr = true; + talloc_free(conn->in); domain->interface->req_cons = domain->interface->req_prod = 0; domain->interface->rsp_cons = domain->interface->rsp_prod = 0; diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index 423fd3e..d314cd5 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -209,8 +209,8 @@ int do_transaction_end(struct connection *conn, struct buffered_data *in) list_del(&trans->list); conn->transaction_started--; - /* Attach transaction to arg for auto-cleanup */ - talloc_steal(arg, trans); + /* Attach transaction to in for auto-cleanup */ + talloc_steal(in, trans); if (streq(arg, "T")) { /* FIXME: Merge, rather failing on any change. */ diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c index e1146ed..94251db 100644 --- a/tools/xenstore/xenstored_watch.c +++ b/tools/xenstore/xenstored_watch.c @@ -164,7 +164,7 @@ int do_watch(struct connection *conn, struct buffered_data *in) /* check if valid event */ } else { relative = !strstarts(vec[0], "/"); - vec[0] = canonicalize(conn, vec[0]); + vec[0] = canonicalize(conn, in, vec[0]); if (!is_valid_nodename(vec[0])) return EINVAL; } @@ -209,7 +209,7 @@ int do_unwatch(struct connection *conn, struct buffered_data *in) if (get_strings(in, vec, ARRAY_SIZE(vec)) != ARRAY_SIZE(vec)) return EINVAL; - node = canonicalize(conn, vec[0]); + node = canonicalize(conn, in, vec[0]); list_for_each_entry(watch, &conn->watches, list) { if (streq(watch->node, node) && streq(watch->token, vec[1])) { list_del(&watch->list);