Xen devel - Jan 2008 - linux: {start, stop}_hz_timer() not really affecting periodic timer?

Shouldn''t these two functions call VCPUOP_set_periodic_timer/
VCPUOP_stop_periodic_timer to actually do what their names promise?

And shouldn''t the call to VCPUOP_set_singleshot_timer permit -ETIME
without BUG()?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

No, no, and no. :-)

start/stop_hz_timer() refer to Linux''s own hz ticker. Xen does not
deliver
periodic ticks when a guest is descheduled, so the Xen side of things is
implicitly handled already. There is no need for start/stop_hz_timer to
execute hypercalls to enact this.

The call to VCPUOP_set_singleshot_timer cannot return -ETIME because the
kernel does not specify the VCPU_SSHOTTMR_future flag.

All this assume you are looking at linux-2.6.18-xen.hg.

 -- Keir

On 16/1/08 14:32, "Jan Beulich" <jbeulich@novell.com> wrote:
> Shouldn''t these two functions call VCPUOP_set_periodic_timer/
> VCPUOP_stop_periodic_timer to actually do what their names promise?
> 
> And shouldn''t the call to VCPUOP_set_singleshot_timer permit
-ETIME
> without BUG()?
> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 16.01.08 15:37
>>>
>No, no, and no. :-)
>
>start/stop_hz_timer() refer to Linux''s own hz ticker. Xen does not
deliver
>periodic ticks when a guest is descheduled, so the Xen side of things is
>implicitly handled already. There is no need for start/stop_hz_timer to
>execute hypercalls to enact this.
Okay, okay, I didn''t pay attention to this. But then
VCPU_stop_periodic_timer seems a rather academic operation?
>The call to VCPUOP_set_singleshot_timer cannot return -ETIME because the
>kernel does not specify the VCPU_SSHOTTMR_future flag.
I noticed this after pushing the send button. Nevertheless, the whole
construct in stop_hz_timer() seems to assume that it is called with
interrupts disabled, which might be the case now but nothing enforces
xen_safe_halt() to only be called in such contexts... For that reason it
would seem safer to set the flag, check for -ETIME, and avoid
HYPERVISOR_block() altogether in that case.
>All this assume you are looking at linux-2.6.18-xen.hg.
I am, with one eye.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/1/08 15:50, "Jan Beulich" <jbeulich@novell.com> wrote:
>> start/stop_hz_timer() refer to Linux''s own hz ticker. Xen does
not deliver
>> periodic ticks when a guest is descheduled, so the Xen side of things
is
>> implicitly handled already. There is no need for start/stop_hz_timer to
>> execute hypercalls to enact this.
> 
> Okay, okay, I didn''t pay attention to this. But then
> VCPU_stop_periodic_timer seems a rather academic operation?
The default periodic timer is 10ms. If a guest does not want a periodic
timer at all, it can use VCPU_stop_periodic_timer. Clearly this is not
applicable to Linux.
>> The call to VCPUOP_set_singleshot_timer cannot return -ETIME because
the
>> kernel does not specify the VCPU_SSHOTTMR_future flag.
> 
> I noticed this after pushing the send button. Nevertheless, the whole
> construct in stop_hz_timer() seems to assume that it is called with
> interrupts disabled, which might be the case now but nothing enforces
> xen_safe_halt() to only be called in such contexts... For that reason it
> would seem safer to set the flag, check for -ETIME, and avoid
> HYPERVISOR_block() altogether in that case.
If the time is in the past then the singleshot timer will fire immediately.
So you''ll take a slower path than necessary, but the code as-is will
work
fine.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> The call to VCPUOP_set_singleshot_timer cannot return -ETIME
because the
>>> kernel does not specify the VCPU_SSHOTTMR_future flag.
>> 
>> I noticed this after pushing the send button. Nevertheless, the whole
>> construct in stop_hz_timer() seems to assume that it is called with
>> interrupts disabled, which might be the case now but nothing enforces
>> xen_safe_halt() to only be called in such contexts... For that reason
it
>> would seem safer to set the flag, check for -ETIME, and avoid
>> HYPERVISOR_block() altogether in that case.
>
>If the time is in the past then the singleshot timer will fire immediately.
>So you''ll take a slower path than necessary, but the code as-is
will work
>fine.
Immediately would mean to me that it would fire on the return path
from VCPUOP_set_singleshot_timer, so HYPERVISOR_block() would
not (necessarily) find any pending events and hence block when it
shouldn''t. Or am I missing some magic by which this is being avoided?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/1/08 16:36, "Jan Beulich" <jbeulich@novell.com> wrote:
>> If the time is in the past then the singleshot timer will fire
immediately.
>> So you''ll take a slower path than necessary, but the code
as-is will work
>> fine.
> 
> Immediately would mean to me that it would fire on the return path
> from VCPUOP_set_singleshot_timer, so HYPERVISOR_block() would
> not (necessarily) find any pending events and hence block when it
> shouldn''t. Or am I missing some magic by which this is being
avoided?
The function should only be called with interrupts disabled.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 16.01.08 17:46
>>>
>On 16/1/08 16:36, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>>> If the time is in the past then the singleshot timer will fire
immediately.
>>> So you''ll take a slower path than necessary, but the code
as-is will work
>>> fine.
>> 
>> Immediately would mean to me that it would fire on the return path
>> from VCPUOP_set_singleshot_timer, so HYPERVISOR_block() would
>> not (necessarily) find any pending events and hence block when it
>> shouldn''t. Or am I missing some magic by which this is being
avoided?
>
>The function should only be called with interrupts disabled.
That goes back to what I stated first - there''s nothing really
requiring
this, it just happens to be that way at present.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/1/08 17:17, "Jan Beulich" <jbeulich@novell.com> wrote:
>>> Immediately would mean to me that it would fire on the return path
>>> from VCPUOP_set_singleshot_timer, so HYPERVISOR_block() would
>>> not (necessarily) find any pending events and hence block when it
>>> shouldn''t. Or am I missing some magic by which this is
being avoided?
>> 
>> The function should only be called with interrupts disabled.
> 
> That goes back to what I stated first - there''s nothing really
requiring
> this, it just happens to be that way at present.
If you don''t do this then the function doesn''t work properly,
so in that
sense it is a requirement! If you want it more explicit add a comment or
maybe even a BUG_ON().

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel