Re: [Xen-devel] [PATCH v5 12/21] xen/arm: move is_vcpu_running function to sched.h

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

Hi Julien, Stefano, Jan,

On Mon, Jun 16, 2014 at 2:04 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 15.06.14 at 18:26, <julien.grall@xxxxxxxxxx> wrote:
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -826,6 +826,22 @@ void watchdog_domain_destroy(struct domain *d);
>>>   #define need_iommu(d)    (0)
>>>   #endif
>>>
>>> +static inline int is_vcpu_running(struct domain *d, int vcpuid)
>>> +{
>>> +    struct vcpu *v;
>>> +
>>> +    if ( vcpuid >= d->max_vcpus )
>>> +        return 0;
>>> +
>>> +    v = d->vcpu[vcpuid];
>>> +    if ( v == NULL )
>>> +        return 0;
>>> +    if (test_bit(_VPF_down, &v->pause_flags) )
>>> +        return 0;
>>> +
>>> +    return 1;
>>> +}
>
> If this function was to become common code, you'd have to not only
> justify it (as requested by Julien), but also
> - name it according to its function (VPF_down not set doesn't mean
>   "running", it merely means "up")

IMO, is_vcpu_running() usage in vgic.c is just to check if vcpu is runnable
and not down. So checking apart from VPF_down is enough.
If so, we should change the name of this function

> - make it properly check d->vcpu != NULL alongside the other checks
> - returning a boolean value it should have return type bool_t
> - "d" and "v" only ever used for read accesses both pointers should
>   get const-qualified
> - fix the one coding style violation in the last if().
>
> Jan
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel