| Message ID | 1495548778-10744-1-git-send-email-anshul.makkar@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 23.05.17 at 16:12, <anshul.makkar@citrix.com> wrote: > The condition: if there is a space in the ring then wait for the producer > to fill the ring also evaluates to true even if the ring if full. It > leads to a deadlock where producer is waiting for consumer > to consume the items and consumer is waiting for producer to fill the ring. > > Fix for the issue: check if the ring is full and then break from > the loop to consume the items from the ring. > eg. case: prod = 1272, cons = 248. > > Signed-off-by: Anshul Makkar <anshul.makkar@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Julien, do you want to consider this for 4.9? Jan
On 23/05/17 16:14, Jan Beulich wrote: >>>> On 23.05.17 at 16:12, <anshul.makkar@citrix.com> wrote: >> The condition: if there is a space in the ring then wait for the producer >> to fill the ring also evaluates to true even if the ring if full. It >> leads to a deadlock where producer is waiting for consumer >> to consume the items and consumer is waiting for producer to fill the ring. >> >> Fix for the issue: check if the ring is full and then break from >> the loop to consume the items from the ring. >> eg. case: prod = 1272, cons = 248. >> >> Signed-off-by: Anshul Makkar <anshul.makkar@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Julien, > > do you want to consider this for 4.9? I'd agree with including this in 4.9 ~Andrew
On 24/05/2017 15:16, Andrew Cooper wrote: > On 23/05/17 16:14, Jan Beulich wrote: >>>>> On 23.05.17 at 16:12, <anshul.makkar@citrix.com> wrote: >>> The condition: if there is a space in the ring then wait for the producer >>> to fill the ring also evaluates to true even if the ring if full. It >>> leads to a deadlock where producer is waiting for consumer >>> to consume the items and consumer is waiting for producer to fill the ring. >>> >>> Fix for the issue: check if the ring is full and then break from >>> the loop to consume the items from the ring. >>> eg. case: prod = 1272, cons = 248. >>> >>> Signed-off-by: Anshul Makkar <anshul.makkar@citrix.com> >> Reviewed-by: Jan Beulich <jbeulich@suse.com> >> >> Julien, >> >> do you want to consider this for 4.9? > > I'd agree with including this in 4.9 Release-acked-by: Julien Grall <julien.grall@arm.com> Cheers,
diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c index 448157d..2b89a56 100644 --- a/tools/firmware/hvmloader/xenbus.c +++ b/tools/firmware/hvmloader/xenbus.c @@ -141,7 +141,19 @@ static void ring_read(char *data, uint32_t len) /* Don't overrun the producer pointer */ while ( (part = MASK_XENSTORE_IDX(rings->rsp_prod - rings->rsp_cons)) == 0 ) + { + /* + * Don't wait for producer to fill the ring if it is already full. + * Condition happens when you write string > 1K into the ring. + * eg case prod=1272 cons=248. + */ + if ( rings->rsp_prod - rings->rsp_cons == XENSTORE_RING_SIZE ) + { + part = XENSTORE_RING_SIZE; + break; + } ring_wait(); + } /* Don't overrun the end of the ring */ if ( part > (XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(rings->rsp_cons)) ) part = XENSTORE_RING_SIZE - MASK_XENSTORE_IDX(rings->rsp_cons);
The condition: if there is a space in the ring then wait for the producer to fill the ring also evaluates to true even if the ring if full. It leads to a deadlock where producer is waiting for consumer to consume the items and consumer is waiting for producer to fill the ring. Fix for the issue: check if the ring is full and then break from the loop to consume the items from the ring. eg. case: prod = 1272, cons = 248. Signed-off-by: Anshul Makkar <anshul.makkar@citrix.com> --- v2: * resolved the coding style issue. * modified the "if" condition statement to make it simpler. tools/firmware/hvmloader/xenbus.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)