return value checking on multicalls

return value checking on multicalls

[Jan Beulich]

Many places currently don't even check HYPERVISOR_multicall()'s return
value, not to speak of checking the individual status codes. Would it be
acceptable to add an argument to this function to request to either fold
all status values into a global success code, or to force BUG_ON() each
individual status. Or should I rather add a new function with described
behavior? Or are there other suggestions?

Thanks, Jan

Re: return value checking on multicalls

[Keir Fraser]

On 14/3/07 15:55, "Jan Beulich" <jbeulich@novell.com> wrote:

> Many places currently don't even check HYPERVISOR_multicall()'s return
> value, not to speak of checking the individual status codes. Would it be
> acceptable to add an argument to this function to request to either fold
> all status values into a global success code, or to force BUG_ON() each
> individual status. Or should I rather add a new function with described
> behavior? Or are there other suggestions?

I think a separate function would be neater, but I'm not sure what you'd
call it. :-) The general idea seems a good one.

 -- Keir

Re: return value checking on multicalls

[Jeremy Fitzhardinge]

Jan Beulich wrote:
> Many places currently don't even check HYPERVISOR_multicall()'s return
> value, not to speak of checking the individual status codes. Would it be
> acceptable to add an argument to this function to request to either fold
> all status values into a global success code, or to force BUG_ON() each
> individual status. Or should I rather add a new function with described
> behavior? Or are there other suggestions?
>   

In the xen-pvops tree I made a general-purpose hypercall batching
mechanism, so that there's only one place which needs to check the
multicall return.  Its interface is:

/* Multicalls */
struct multicall_space
{
	struct multicall_entry *mc;
	void *args;
};

/* Allocate room for a multicall and its args */
struct multicall_space xen_mc_entry(size_t args);

/* Flush all pending multicalls */
void xen_mc_flush(void);

/* Issue a multicall if we're not in lazy mode */
static inline void xen_mc_issue(void)
{
	if (xen_get_lazy_mode() == PARAVIRT_LAZY_NONE)
		xen_mc_flush();
}


xen_mc_flush() just BUGs if either the multicall hypercall itself fails,
or any of the constituent hypercalls.

    J

Re: return value checking on multicalls

[Jan Beulich]

>>> Jeremy Fitzhardinge <jeremy@goop.org> 14.03.07 17:45 >>>
>Jan Beulich wrote:
>> Many places currently don't even check HYPERVISOR_multicall()'s return
>> value, not to speak of checking the individual status codes. Would it be
>> acceptable to add an argument to this function to request to either fold
>> all status values into a global success code, or to force BUG_ON() each
>> individual status. Or should I rather add a new function with described
>> behavior? Or are there other suggestions?
>
>In the xen-pvops tree I made a general-purpose hypercall batching
>mechanism, so that there's only one place which needs to check the
>multicall return.  Its interface is:
>
>/* Multicalls */
>struct multicall_space
>{
>	struct multicall_entry *mc;
>	void *args;
>};
>
>/* Allocate room for a multicall and its args */
>struct multicall_space xen_mc_entry(size_t args);
>
>/* Flush all pending multicalls */
>void xen_mc_flush(void);
>
>/* Issue a multicall if we're not in lazy mode */
>static inline void xen_mc_issue(void)
>{
>	if (xen_get_lazy_mode() == PARAVIRT_LAZY_NONE)
>		xen_mc_flush();
>}
>
>
>xen_mc_flush() just BUGs if either the multicall hypercall itself fails,
>or any of the constituent hypercalls.

Hmm, that doesn't seem to fit all cases. There are hypercalls that
return non-errno-like values, and you shouldn't make any
assumptions about these.

Jan

Re: return value checking on multicalls

[Jeremy Fitzhardinge]

Jan Beulich wrote:
> Hmm, that doesn't seem to fit all cases. There are hypercalls that
> return non-errno-like values, and you shouldn't make any
> assumptions about these.

It's been a nice simplification in my work, but I haven't used it with
anything where success > 0, or where I've even cared about results other
than failure.  And the advantage is that the incremental effort of
batching a series of calls is very small.

I just thought it might be useful.  Perhaps the interface could be
extended to be more useful in the cases you're looking at.

    J