From 5d0bd0712d71aa993682a86f6afe5a112a750db8 Mon Sep 17 00:00:00 2001 From: Anton Kortunov Date: Tue, 24 Nov 2015 18:42:46 +0300 Subject: [PATCH 1/3] Core: bypass write to pipe for local transactions Reworked dnet_io_req_queue function to pass io_req struct directly to the proper IO pool, eliminating write/read through pipe in net pool. --- library/net.c | 95 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 10 deletions(-) diff --git a/library/net.c b/library/net.c index fef754cf1..da6c0bf0e 100644 --- a/library/net.c +++ b/library/net.c @@ -171,14 +171,28 @@ void dnet_state_clean(struct dnet_net_state *st) dnet_log(st->n, DNET_LOG_NOTICE, "Cleaned state %s, transactions freed: %d", dnet_state_dump_addr(st), num); } -static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dnet_io_req *orig) +/* + * This function makes a copy of io_req to transfer memory ownership to another thread. + * If target thread is net thread we can just make a copy and propertly fill fd params. + * If target thread is io thread (bypass for local transaction on server side, read_data flag + * is set to 1) we need to allocate buffer and read fd content info this buffer. + * Result should looks exactly as if was read from network socket. This CPU IO time is spent + * in backend's IO pool. + */ +static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dnet_io_req *orig, int read_data) { void *buf; + size_t len = 0; struct dnet_io_req *r; int offset = 0; int err = 0; - buf = r = malloc(sizeof(struct dnet_io_req) + orig->dsize + orig->hsize); + len = sizeof(struct dnet_io_req) + orig->dsize + orig->hsize; + if (orig->fd >= 0 && orig->fsize && read_data) { + len += orig->fsize; + } + + buf = r = malloc(len); if (!r) { dnet_log(st->n, DNET_LOG_ERROR, "Not enough memory for io req queue fd: %d : %s %d", orig->fd, strerror(-err), err); return NULL; @@ -192,7 +206,10 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn offset = r->hsize; memcpy(r->header, orig->header, r->hsize); - } + } else if (read_data) { + r->header = buf + sizeof(struct dnet_io_req); + r->hsize = 0; + } if (orig->data && orig->dsize) { r->data = buf + sizeof(struct dnet_io_req) + offset; @@ -203,13 +220,33 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn } if (orig->fd >= 0 && orig->fsize) { - r->fd = orig->fd; - r->on_exit = orig->on_exit; - r->local_offset = orig->local_offset; - r->fsize = orig->fsize; + if (read_data) { + if (r->data == NULL) { + r->data = buf + sizeof(struct dnet_io_req) + offset; + } + + err = dnet_read_ll(orig->fd, r->data + r->dsize, orig->fsize, orig->local_offset); + if (err) { + dnet_log(st->n, DNET_LOG_ERROR, "Error while reading data: %s %d", strerror(-err), err); + goto err_out_free; + } + + r->dsize += orig->fsize; + r->on_exit = orig->on_exit; + + } else { + r->fd = orig->fd; + r->on_exit = orig->on_exit; + r->local_offset = orig->local_offset; + r->fsize = orig->fsize; + } } return r; + +err_out_free: + free(r); + return NULL; } /* @@ -221,7 +258,41 @@ static int dnet_io_req_queue(struct dnet_net_state *st, struct dnet_io_req *orig int err = 0; struct dnet_io_req *r; - r = dnet_io_req_copy(st, orig); + /* + * If destination is local node there is no need to copy it via net thread. + * Here it could be sheduled to the proper IO pool directly. + * If io_req hold fd instead of data it should be read here. + */ + if (st == st->n->st) { + dnet_log(st->n, DNET_LOG_DEBUG, "Sending to local state: data: 0x%lx data_size: %d", (unsigned long)orig->data, (int)orig->dsize); + + r = dnet_io_req_copy(st, orig, 1); + if (!r) { + err = -ENOMEM; + goto err_out_exit; + } + + /* + * Fixup request. + * r->header and r->data are just a pointers to the same buffer. + * Here r->header could be any size, it depend on function that created io_req. + * In net thread r->hsize is ALWAYS sizeof(struct dnet_cmd) and r->data points + * exactly after struct dnet_cmd struct. We need to modify our copy to the same layout + * because dnet_schedule_io requires it. + */ + if (r->hsize != sizeof(struct dnet_cmd)) { + r->data = r->header + sizeof(struct dnet_cmd); + r->dsize += r->hsize - sizeof(struct dnet_cmd); + r->hsize = sizeof(struct dnet_cmd); + } + + r->st = dnet_state_get(st); + dnet_schedule_io(st->n, r); + + return 0; + } + + r = dnet_io_req_copy(st, orig, 0); if (!r) { err = -ENOMEM; goto err_out_exit; @@ -336,8 +407,12 @@ ssize_t dnet_send(struct dnet_net_state *st, void *data, uint64_t size) struct dnet_io_req r; memset(&r, 0, sizeof(r)); - r.data = data; - r.dsize = size; + r.header = data; + r.hsize = sizeof(struct dnet_cmd); + if (size > r.hsize) { + r.data = data + r.hsize; + r.dsize = size - r.hsize; + } r.fd = -1; return dnet_io_req_queue(st, &r); From f73ff71668fa283379e4749c3d835a037b96d27b Mon Sep 17 00:00:00 2001 From: Anton Kortunov Date: Wed, 9 Dec 2015 13:45:26 +0300 Subject: [PATCH 2/3] Fixed typos --- library/net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/net.c b/library/net.c index da6c0bf0e..2da620787 100644 --- a/library/net.c +++ b/library/net.c @@ -173,7 +173,7 @@ void dnet_state_clean(struct dnet_net_state *st) /* * This function makes a copy of io_req to transfer memory ownership to another thread. - * If target thread is net thread we can just make a copy and propertly fill fd params. + * If target thread is net thread we can just make a copy and properly fill fd params. * If target thread is io thread (bypass for local transaction on server side, read_data flag * is set to 1) we need to allocate buffer and read fd content info this buffer. * Result should looks exactly as if was read from network socket. This CPU IO time is spent @@ -260,7 +260,7 @@ static int dnet_io_req_queue(struct dnet_net_state *st, struct dnet_io_req *orig /* * If destination is local node there is no need to copy it via net thread. - * Here it could be sheduled to the proper IO pool directly. + * Here it could be scheduled to the proper IO pool directly. * If io_req hold fd instead of data it should be read here. */ if (st == st->n->st) { From c4ef2777c2cd1faed4e74bc5915a39f05b9e23af Mon Sep 17 00:00:00 2001 From: Anton Kortunov Date: Wed, 9 Dec 2015 14:38:46 +0300 Subject: [PATCH 3/3] Core: request fixup moved to dnet_io_req_copy To isolate work with dnet_io_req data pointers request fixup was moved to dnet_io_req_copy function from dnet_io_req_queue. --- library/net.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/library/net.c b/library/net.c index 2da620787..b1099416d 100644 --- a/library/net.c +++ b/library/net.c @@ -174,12 +174,12 @@ void dnet_state_clean(struct dnet_net_state *st) /* * This function makes a copy of io_req to transfer memory ownership to another thread. * If target thread is net thread we can just make a copy and properly fill fd params. - * If target thread is io thread (bypass for local transaction on server side, read_data flag + * If target thread is io thread (bypass for local transaction on server side, bypass flag * is set to 1) we need to allocate buffer and read fd content info this buffer. * Result should looks exactly as if was read from network socket. This CPU IO time is spent * in backend's IO pool. */ -static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dnet_io_req *orig, int read_data) +static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dnet_io_req *orig, int bypass) { void *buf; size_t len = 0; @@ -188,7 +188,7 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn int err = 0; len = sizeof(struct dnet_io_req) + orig->dsize + orig->hsize; - if (orig->fd >= 0 && orig->fsize && read_data) { + if (orig->fd >= 0 && orig->fsize && bypass) { len += orig->fsize; } @@ -206,7 +206,7 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn offset = r->hsize; memcpy(r->header, orig->header, r->hsize); - } else if (read_data) { + } else if (bypass) { r->header = buf + sizeof(struct dnet_io_req); r->hsize = 0; } @@ -220,7 +220,7 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn } if (orig->fd >= 0 && orig->fsize) { - if (read_data) { + if (bypass) { if (r->data == NULL) { r->data = buf + sizeof(struct dnet_io_req) + offset; } @@ -242,6 +242,22 @@ static struct dnet_io_req *dnet_io_req_copy(struct dnet_net_state *st, struct dn } } + if (bypass) { + /* + * Fixup request. + * r->header and r->data are just a pointers to the same buffer. + * Here r->header could be any size, it depend on function that created io_req. + * In net thread r->hsize is ALWAYS sizeof(struct dnet_cmd) and r->data points + * exactly after struct dnet_cmd struct. We need to modify our copy to the same layout + * because dnet_schedule_io requires it. + */ + if (r->hsize != sizeof(struct dnet_cmd)) { + r->data = r->header + sizeof(struct dnet_cmd); + r->dsize += r->hsize - sizeof(struct dnet_cmd); + r->hsize = sizeof(struct dnet_cmd); + } + } + return r; err_out_free: @@ -272,20 +288,6 @@ static int dnet_io_req_queue(struct dnet_net_state *st, struct dnet_io_req *orig goto err_out_exit; } - /* - * Fixup request. - * r->header and r->data are just a pointers to the same buffer. - * Here r->header could be any size, it depend on function that created io_req. - * In net thread r->hsize is ALWAYS sizeof(struct dnet_cmd) and r->data points - * exactly after struct dnet_cmd struct. We need to modify our copy to the same layout - * because dnet_schedule_io requires it. - */ - if (r->hsize != sizeof(struct dnet_cmd)) { - r->data = r->header + sizeof(struct dnet_cmd); - r->dsize += r->hsize - sizeof(struct dnet_cmd); - r->hsize = sizeof(struct dnet_cmd); - } - r->st = dnet_state_get(st); dnet_schedule_io(st->n, r);