From mboxrd@z Thu Jan 1 00:00:00 1970 From: Samuel Thibault Subject: Re: [PATCH] oxenstored: fix short-write issue Date: Wed, 28 Oct 2015 14:47:32 +0100 Message-ID: <20151028134732.GF2924@var.bordeaux.inria.fr> References: <1445965809-5144-1-git-send-email-wei.liu2@citrix.com> <562FB2A3.8070603@citrix.com> <20151027172810.GQ2483@var.bordeaux.inria.fr> <562FB4F9.20100@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="r5Pyd7+fXNt84Ff3" Content-Transfer-Encoding: 8bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZrR4g-0001eD-EL for xen-devel@lists.xenproject.org; Wed, 28 Oct 2015 13:47:34 +0000 Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Scott Cc: Andrew Cooper , Ian Campbell , Wei Liu , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org --r5Pyd7+fXNt84Ff3 Content-Type: text/plain; charset=utf-8 Content-Length: 2195 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline David Scott, le Wed 28 Oct 2015 13:34:10 +0000, a =C3=A9crit : > However I believe the =E2=80=98suspicious=E2=80=99 OCaml patch fixes the specific issue (=E2=80=94 or have I missed something=3F). It does. > Does anyone have time to prototype what a C-level fix would look like=3F Here is a try, untested though. Signed-off-by: Samuel Thibault diff --git a/tools/ocaml/libs/xb/xs_ring_stubs.c b/tools/ocaml/libs/xb/xs_ring_stubs.c index fd561a2..5c771d6 100644 --- a/tools/ocaml/libs/xb/xs_ring_stubs.c +++ b/tools/ocaml/libs/xb/xs_ring_stubs.c @@ -99,12 +99,12 @@ CAMLprim value ml_interface_write(value ml_interface, int result; struct xenstore_domain_interface *intf =3D interface->addr; - XENSTORE_RING_IDX cons, prod; + XENSTORE_RING_IDX cons, old_prod, prod; int can_write; uint32_t connection; cons =3D *(volatile uint32_t*)&intf->rsp_cons; - prod =3D *(volatile uint32_t*)&intf->rsp_prod; + old_prod =3D prod =3D *(volatile uint32_t*)&intf->rsp_prod; connection =3D *(volatile uint32_t*)&intf->connection; if (connection !=3D XENSTORE_CONNECTED) @@ -116,15 +116,30 @@ CAMLprim value ml_interface_write(value ml_interface, goto exit; } if (MASK_XENSTORE_IDX(prod) >=3D MASK_XENSTORE_IDX(cons)) + { can_write =3D XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(prod); - else + if (can_write < len) + { + /* Not enough room at end of ring. Copy what we can first. */ + memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, can_write); + prod +=3D can_write; + len -=3D can_write; + buffer +=3D can_write; + + /* We are now back at beginning of ring. */ + can_write =3D MASK_XENSTORE_IDX(cons) - MASK_XENSTORE_IDX(prod); + } + } + else can_write =3D MASK_XENSTORE_IDX(cons) - MASK_XENSTORE_IDX(prod); if (can_write < len) + /* Real short write: not enough room. */ len =3D can_write; memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, len); + prod +=3D len; xen_mb(); - intf->rsp_prod +=3D len; - result =3D len; + intf->rsp_prod =3D prod; + result =3D prod - old_prod; exit: ml_result =3D Val_int(result); CAMLreturn(ml_result); --r5Pyd7+fXNt84Ff3 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=patch diff --git a/tools/ocaml/libs/xb/xs_ring_stubs.c b/tools/ocaml/libs/xb/xs_ring_stubs.c index fd561a2..5c771d6 100644 --- a/tools/ocaml/libs/xb/xs_ring_stubs.c +++ b/tools/ocaml/libs/xb/xs_ring_stubs.c @@ -99,12 +99,12 @@ CAMLprim value ml_interface_write(value ml_interface, int result; struct xenstore_domain_interface *intf = interface->addr; - XENSTORE_RING_IDX cons, prod; + XENSTORE_RING_IDX cons, old_prod, prod; int can_write; uint32_t connection; cons = *(volatile uint32_t*)&intf->rsp_cons; - prod = *(volatile uint32_t*)&intf->rsp_prod; + old_prod = prod = *(volatile uint32_t*)&intf->rsp_prod; connection = *(volatile uint32_t*)&intf->connection; if (connection != XENSTORE_CONNECTED) @@ -116,15 +116,30 @@ CAMLprim value ml_interface_write(value ml_interface, goto exit; } if (MASK_XENSTORE_IDX(prod) >= MASK_XENSTORE_IDX(cons)) + { can_write = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(prod); - else + if (can_write < len) + { + /* Not enough room at end of ring. Copy what we can first. */ + memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, can_write); + prod += can_write; + len -= can_write; + buffer += can_write; + + /* We are now back at beginning of ring. */ + can_write = MASK_XENSTORE_IDX(cons) - MASK_XENSTORE_IDX(prod); + } + } + else can_write = MASK_XENSTORE_IDX(cons) - MASK_XENSTORE_IDX(prod); if (can_write < len) + /* Real short write: not enough room. */ len = can_write; memcpy(intf->rsp + MASK_XENSTORE_IDX(prod), buffer, len); + prod += len; xen_mb(); - intf->rsp_prod += len; - result = len; + intf->rsp_prod = prod; + result = prod - old_prod; exit: ml_result = Val_int(result); CAMLreturn(ml_result); --r5Pyd7+fXNt84Ff3 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --r5Pyd7+fXNt84Ff3--