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

Groups > linux.kernel > #1555692

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

Show all headers | View raw

On 10/01/17 17:36, Boris Ostrovsky wrote:
> 
>>>> +static int process_msg(void)
>>>> +{
>>>> +	static struct xs_thread_state_read state;
>>>> +	struct xb_req_data *req;
>>>> +	int err;
>>>> +	unsigned int len;
>>>> +
>>>> +	if (!state.in_msg) {
>>>> +		state.in_msg = true;
>>>> +		state.in_hdr = true;
>>>> +		state.used = 0;
>>>> +
>>>> +		/*
>>>> +		 * We must disallow save/restore while reading a message.
>>>> +		 * A partial read across s/r leaves us out of sync with
>>>> +		 * xenstored.
>>>> +		 */
>>>> +		mutex_lock(&xs_response_mutex);
>>>> +
>>>> +		if (!xb_data_to_read()) {
>>>> +			/* We raced with save/restore: pending data 'gone'. */
>>>> +			mutex_unlock(&xs_response_mutex);
>>>> +			state.in_msg = false;
> 
> Just noticed: should in_hdr be set to false here as well?

Doesn't matter: It is valid only if in_msg is true.

> 
>>>> +			return 0;
>>>> +		}
> 
> Or set it to true here.
> 
>>>> +	}
>>>> +
>>>> +	if (state.in_hdr) {
>>>> +		if (state.used != sizeof(state.msg)) {
>>>> +			err = xb_read((void *)&state.msg + state.used,
>>>> +				      sizeof(state.msg) - state.used);
>>>> +			if (err < 0)
>>>> +				goto out;
>>>> +			state.used += err;
>>>> +			if (state.used != sizeof(state.msg))
>>>> +				return 0;
>>> Would it be possible to do locking at the caller? I understand that you
>>> are trying to hold the lock across multiple invocations of this function
>>> but it feels somewhat counter-intuitive and bug-prone.
>> I think that would be difficult.
>>
>>> If it's not possible then at least please add a comment explaining
>>> locking algorithm.
>> Okay. Something like:
>>
>> /*
>>  * xs_response_mutex is locked as long as we are processing one
>>  * message. state.in_msg will be true as long as we are holding the
>>  * lock in process_msg().
> 
> 
> Then in_msg is the same as mutex_is_locked(&xs_response_mutex). And if
> so, do we really need it?

Yes. xs_response_mutex is used in suspend path, too.


Juergen

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

Thread

[PATCH 0/3] xen: optimize xenbus performance Juergen Gross <jgross@suse.com> - 2017-01-06 16:10 +0100
  [PATCH 2/3] xen: modify xenstore watch event interface Juergen Gross <jgross@suse.com> - 2017-01-06 16:10 +0100
    RE: [PATCH 2/3] xen: modify xenstore watch event interface Paul Durrant <Paul.Durrant@citrix.com> - 2017-01-06 16:40 +0100
    Re: [PATCH 2/3] xen: modify xenstore watch event interface Wei Liu <wei.liu2@citrix.com> - 2017-01-06 17:30 +0100
    Re: [PATCH 2/3] xen: modify xenstore watch event interface Roger Pau Monné <roger.pau@citrix.com> - 2017-01-06 17:40 +0100
    Re: [PATCH 2/3] xen: modify xenstore watch event interface Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-06 23:40 +0100
      Re: [PATCH 2/3] xen: modify xenstore watch event interface Juergen Gross <jgross@suse.com> - 2017-01-09 08:20 +0100
  Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-09 22:20 +0100
    Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Juergen Gross <jgross@suse.com> - 2017-01-10 07:20 +0100
      Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-10 17:40 +0100
      Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-10 17:40 +0100
        Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Juergen Gross <jgross@suse.com> - 2017-01-10 17:50 +0100
  Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-10 20:20 +0100
  Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-11 00:00 +0100
    Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Juergen Gross <jgross@suse.com> - 2017-01-11 06:30 +0100
      Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-01-11 16:40 +0100
        Re: [PATCH 3/3] xen: optimize xenbus driver for multiple concurrent  xenstore accesses Juergen Gross <jgross@suse.com> - 2017-01-11 18:00 +0100

csiph-web