Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1576293

Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple concurrent xenstore accesses

From Juergen Gross <jgross@suse.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple concurrent xenstore accesses
Date 2017-02-08 07:30 +0100
Message-ID <t8tVo-5al-15@gated-at.bofh.it> (permalink)
References (2 earlier) <t2S0q-1AD-15@gated-at.bofh.it> <t39DY-4W7-29@gated-at.bofh.it> <t3c8O-6zx-11@gated-at.bofh.it> <t8idA-5XI-29@gated-at.bofh.it> <t8mAy-mn-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 07/02/17 23:39, Boris Ostrovsky wrote:
> On 02/07/2017 12:51 PM, Boris Ostrovsky wrote:
>> On 01/24/2017 11:23 AM, Juergen Gross wrote:
>>> On 24/01/17 14:47, Boris Ostrovsky wrote:
>>>> On 01/23/2017 01:59 PM, Boris Ostrovsky wrote:
>>>>> On 01/23/2017 05:09 AM, Juergen Gross wrote:
>>>>>> Handling of multiple concurrent Xenstore accesses through xenbus driver
>>>>>> either from the kernel or user land is rather lame today: xenbus is
>>>>>> capable to have one access active only at one point of time.
>>>>>>
>>>>>>
>>>> This patch appears to break save/restore:
>>> Hmm, tried multiple times, but I can't reproduce this issue.
>>>
>>> Anything special in the setup? I tried a 64 bit pv guest and did
>>> "xl save".
>>>
>>> Do I have to run some load in parallel?
>> Any luck reproducing this? I am still failing the test on dumpdata but I
>> couldn't reproduce it on another system.
> 
> 
> The problem appears to be xs_state_users being non-zero when we call
> xs_suspend_enter().
> 
> From what I understand this is caused by xs_request_exit() not
> decrementing it when closing a transaction. This seems to be happening
> when XS_TRANSACTION_END transaction returns XS_ERROR (I haven't traced
> what causes this error but it doesn't appear to cause any visible harm).

Aah, of course: this will happen for a transaction failing due to a
conflict (EAGAIN case).

> Does the patch below make sense?

As the xenbus driver is checking for the transaction id to be valid this
is okay, I think.

I have noticed another problem, though: a user client mixing Xenstore
accesses with and without transactions could be blocked when doing a
non-transactional access hindering it from completing the transaction.

I'll send an updated version including your fix.

> 
> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
> index e62cb09..ffd5fac 100644
> --- a/drivers/xen/xenbus/xenbus_xs.c
> +++ b/drivers/xen/xenbus/xenbus_xs.c
> @@ -140,7 +140,7 @@ void xs_request_exit(struct xb_req_data *req)
>         spin_lock(&xs_state_lock);
>         xs_state_users--;
>         if ((req->type == XS_TRANSACTION_START && req->msg.type ==
> XS_ERROR) ||
> -           req->msg.type == XS_TRANSACTION_END)
> +           req->type == XS_TRANSACTION_END)
>                 xs_state_users--;
>         spin_unlock(&xs_state_lock);
> 
> 
> I ran a few tests on dumpdata and they completed successfully. I'll keep
> this for the overnight runs too, with a different Xen version.

Thanks.


Juergen

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple  concurrent xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-02-07 19:00 +0100
  Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple  concurrent xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-02-07 23:40 +0100
    Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple  concurrent xenstore accesses Juergen Gross <jgross@suse.com> - 2017-02-08 07:30 +0100

csiph-web