Re: [PATCH v6 1/2] xen: introduce vcpu_block, use it instead of do_block

[Keir Fraser <keir.xen@gmail.com>]

On 19/04/2013 10:34, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
wrote:

> On Thu, 18 Apr 2013, Keir Fraser wrote:
>> On 18/04/2013 19:49, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
>> wrote:
>> 
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> 
>> Why? Patch 2/2 only passes current, and the changes here still leave
>> vcpu_block() implicitly assuming v==current (through use of local_event_*
>> calls). You may as well leave this as do_block(void).
>> 
> 
> Just cosmetics: I was trying to make it look like the corresponding
> function of vcpu_unblock.

Three acceptable options:
 - Leave it as it is
 - Rename it to vcpu_block_current(void)
 - Rename to vcpu_block(vcpu *v) but ASSERT(v == current) as a pre-condition

I like options 1 & 2 best. I'm okay with option 3.

 -- Keir

> 
>>> ---
>>>  xen/common/schedule.c   |   14 ++++++--------
>>>  xen/include/xen/sched.h |    1 +
>>>  2 files changed, 7 insertions(+), 8 deletions(-)
>>> 
>>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
>>> index c1cd3d0..89e4b7f 100644
>>> --- a/xen/common/schedule.c
>>> +++ b/xen/common/schedule.c
>>> @@ -676,11 +676,9 @@ int vcpu_set_affinity(struct vcpu *v, const cpumask_t
>>> *affinity)
>>>      return 0;
>>>  }
>>>  
>>> -/* Block the currently-executing domain until a pertinent event occurs. */
>>> -static long do_block(void)
>>> +/* Block a vcpu until a pertinent event occurs. */
>>> +void vcpu_block(struct vcpu *v)
>>>  {
>>> -    struct vcpu *v = current;
>>> -
>>>      local_event_delivery_enable();
>>>      set_bit(_VPF_blocked, &v->pause_flags);
>>>  
>>> @@ -694,8 +692,6 @@ static long do_block(void)
>>>          TRACE_2D(TRC_SCHED_BLOCK, v->domain->domain_id, v->vcpu_id);
>>>          raise_softirq(SCHEDULE_SOFTIRQ);
>>>      }
>>> -
>>> -    return 0;
>>>  }
>>>  
>>>  static long do_poll(struct sched_poll *sched_poll)
>>> @@ -870,7 +866,8 @@ long do_sched_op_compat(int cmd, unsigned long arg)
>>>  
>>>      case SCHEDOP_block:
>>>      {
>>> -        ret = do_block();
>>> +        vcpu_block(current);
>>> +        ret = 0;
>>>          break;
>>>      }
>>>  
>>> @@ -907,7 +904,8 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
>>> arg)
>>>  
>>>      case SCHEDOP_block:
>>>      {
>>> -        ret = do_block();
>>> +        vcpu_block(current);
>>> +        ret = 0;
>>>          break;
>>>      }
>>>  
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index ad971d2..e74f0d7 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -691,6 +691,7 @@ static inline int vcpu_runnable(struct vcpu *v)
>>>  }
>>>  
>>>  void vcpu_unblock(struct vcpu *v);
>>> +void vcpu_block(struct vcpu *v);
>>>  void vcpu_pause(struct vcpu *v);
>>>  void vcpu_pause_nosync(struct vcpu *v);
>>>  void domain_pause(struct domain *d);
>> 
>>