All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cxenstored: correct calculation of data/space in the ring
@ 2015-11-16 12:53 Wei Liu
  2015-11-16 18:01 ` Ian Jackson
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2015-11-16 12:53 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Wei Liu, Ian Campbell, Andrew Cooper

The cxenstored implementation can't handle cross ring boundary read and
write. It gets aways with buggy behaviour because upper layer won't
sleep when short-write or short-read occurs.

It would be nice, however, to make the low level routine correct
regardless of what upper layer is doing. This should at least avoid
latent bug should upper layer logic changes.

This patch ports 8a2c11f8 ("tools/ocaml/xb: Correct calculations of
data/space the ring") for oxenstored to cxenstored.

Two functions -- get_{input,output}_chunks -- become unused. Nuke them.
Preserve the behaviour of always sending notification to the other end
when exiting.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/xenstore/xenstored_domain.c | 76 +++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index dcd6581..4c0b87e 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -82,33 +82,12 @@ static bool check_indexes(XENSTORE_RING_IDX cons, XENSTORE_RING_IDX prod)
 	return ((prod - cons) <= XENSTORE_RING_SIZE);
 }
 
-static void *get_output_chunk(XENSTORE_RING_IDX cons,
-			      XENSTORE_RING_IDX prod,
-			      char *buf, uint32_t *len)
-{
-	*len = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(prod);
-	if ((XENSTORE_RING_SIZE - (prod - cons)) < *len)
-		*len = XENSTORE_RING_SIZE - (prod - cons);
-	return buf + MASK_XENSTORE_IDX(prod);
-}
-
-static const void *get_input_chunk(XENSTORE_RING_IDX cons,
-				   XENSTORE_RING_IDX prod,
-				   const char *buf, uint32_t *len)
-{
-	*len = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(cons);
-	if ((prod - cons) < *len)
-		*len = prod - cons;
-	return buf + MASK_XENSTORE_IDX(cons);
-}
-
 static int writechn(struct connection *conn,
-		    const void *data, unsigned int len)
+		    const void *buffer, unsigned int len)
 {
-	uint32_t avail;
-	void *dest;
 	struct xenstore_domain_interface *intf = conn->domain->interface;
 	XENSTORE_RING_IDX cons, prod;
+	int total_space, space;
 
 	/* Must read indexes once, and before anything else, and verified. */
 	cons = intf->rsp_cons;
@@ -120,25 +99,39 @@ static int writechn(struct connection *conn,
 		return -1;
 	}
 
-	dest = get_output_chunk(cons, prod, intf->rsp, &avail);
-	if (avail < len)
-		len = avail;
+	total_space = XENSTORE_RING_SIZE - (prod - cons);
+	if (total_space == 0) {
+		/* No space at all - exit having done nothing. */
+		len = 0;
+		goto exit;
+	} else if (total_space < len)
+		/* Some space - make a partial write */
+		len = total_space;
+
+	space = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(prod);
+	if (len < space)
+		/* Message fits inside the remaining part of the ring */
+		memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, len);
+	else {
+		/* Message wraps around the end of ring. Write both halves. */
+		memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, space);
+		memcpy(intf->rsp, buffer + space, len - space);
+	}
 
-	memcpy(dest, data, len);
 	xen_mb();
 	intf->rsp_prod += len;
 
+exit:
 	xc_evtchn_notify(xce_handle, conn->domain->port);
 
 	return len;
 }
 
-static int readchn(struct connection *conn, void *data, unsigned int len)
+static int readchn(struct connection *conn, void *buffer, unsigned int len)
 {
-	uint32_t avail;
-	const void *src;
 	struct xenstore_domain_interface *intf = conn->domain->interface;
 	XENSTORE_RING_IDX cons, prod;
+	int total_data, data;
 
 	/* Must read indexes once, and before anything else, and verified. */
 	cons = intf->req_cons;
@@ -150,14 +143,29 @@ static int readchn(struct connection *conn, void *data, unsigned int len)
 		return -1;
 	}
 
-	src = get_input_chunk(cons, prod, intf->req, &avail);
-	if (avail < len)
-		len = avail;
+	total_data = prod - cons;
+	if (total_data == 0) {
+		/* No pending data at all. */
+		len = 0;
+		goto exit;
+	} else if (total_data < len)
+		/* Some data - make a partial read. */
+		len = total_data;
+
+	data = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(cons);
+	if (len < data)
+		/* Data within the remaining part of the ring. */
+		memcpy(buffer, intf->req + MASK_XENSTORE_IDX(cons), len);
+	else {
+		/* Data crosses the ring boundary. Read both halves. */
+		memcpy(buffer, intf->req + MASK_XENSTORE_IDX(cons), data);
+		memcpy(buffer + data, intf->req, len - data);
+	}
 
-	memcpy(data, src, len);
 	xen_mb();
 	intf->req_cons += len;
 
+exit:
 	xc_evtchn_notify(xce_handle, conn->domain->port);
 
 	return len;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-24 16:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-16 12:53 [PATCH] cxenstored: correct calculation of data/space in the ring Wei Liu
2015-11-16 18:01 ` Ian Jackson
2015-11-16 18:06   ` Andrew Cooper
2015-11-16 18:09     ` Ian Jackson
2015-11-16 18:36       ` Wei Liu
2015-11-24 16:21         ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.