RE: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

RE: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Christian Limpach]

Jeremy Fitzhardinge wrote:
> Andrew Morton wrote:
> > On Thu, 15 Feb 2007 22:14:45 -0800 Dan Hecht 
> <dhecht@vmware.com> wrote:
> >
> >   
> >>>  config PREEMPT
> >>>  	bool "Preemptible Kernel (Low-Latency Desktop)"
> >>> +	depends on !XEN
> >>>  	help
> >>>  	  This option reduces the latency of the kernel by making
> >>>  	  all kernel code (that is not executing in a critical section)
> >>>
> >>>       
> >
> > Oh, so that's why it doesn't break when CONFIG_PREEMPT=y.  
> In which case
> > that preempt_disable() I spotted is wrong-and-unneeded.
> >
> > Why doesn't Xen work with preemption??
> >   
> 
> I've forgotten the details.  Ian?  Keir?  Steven?  Maybe it 
> can be done.

With CONFIG_PREEMPT, we can have preempted threads reference machine
addresses across save/restore.  After restore, these machine addresses
will be incorrect.

    Christian

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Andrew Morton]

On Fri, 16 Feb 2007 02:00:39 -0800 "Christian Limpach" <Christian.Limpach@xensource.com> wrote:

> Jeremy Fitzhardinge wrote:
> > Andrew Morton wrote:
> > > On Thu, 15 Feb 2007 22:14:45 -0800 Dan Hecht 
> > <dhecht@vmware.com> wrote:
> > >
> > >   
> > >>>  config PREEMPT
> > >>>  	bool "Preemptible Kernel (Low-Latency Desktop)"
> > >>> +	depends on !XEN
> > >>>  	help
> > >>>  	  This option reduces the latency of the kernel by making
> > >>>  	  all kernel code (that is not executing in a critical section)
> > >>>
> > >>>       
> > >
> > > Oh, so that's why it doesn't break when CONFIG_PREEMPT=y.  
> > In which case
> > > that preempt_disable() I spotted is wrong-and-unneeded.
> > >
> > > Why doesn't Xen work with preemption??
> > >   
> > 
> > I've forgotten the details.  Ian?  Keir?  Steven?  Maybe it 
> > can be done.
> 
> With CONFIG_PREEMPT, we can have preempted threads reference machine
> addresses across save/restore.  After restore, these machine addresses
> will be incorrect.
> 

It would help if you could define the terms "save/restore" and "machine
addresses".  One can guess, but there are probably subtleties here..

Are the places where the domU code references machine addresses splattered
all over the code?  If not, they can just be wrapped with
preempt_disable/preempt_enable?

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Keir Fraser]

On 16/2/07 10:09, "Andrew Morton" <akpm@linux-foundation.org> wrote:

> Are the places where the domU code references machine addresses splattered
> all over the code?  If not, they can just be wrapped with
> preempt_disable/preempt_enable?

The main places where machine addresses are 'visible' are any code that
holds a pte_t,pmd_t,pud_t,pgd_t. We hide the machine-to-pseudophysical and
pseudophysical-to-machine translations inside e.g., pte_val() and __pte()
(i.e., constructors and extractors for page table entries). Obviously the
users of these macros are open coded all over the place, quite apart from
the performance cost of sprinkling preempt_{enable,disable} so liberally.

 -- Keir

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Keir Fraser]

On 16/2/07 10:09, "Andrew Morton" <akpm@linux-foundation.org> wrote:

> Are the places where the domU code references machine addresses splattered
> all over the code?  If not, they can just be wrapped with
> preempt_disable/preempt_enable?

The main places where machine addresses are 'visible' are any code that
holds a pte_t,pmd_t,pud_t,pgd_t. We hide the machine-to-pseudophysical and
pseudophysical-to-machine translations inside e.g., pte_val() and __pte()
(i.e., constructors and extractors for page table entries). Obviously the
users of these macros are open coded all over the place, quite apart from
the performance cost of sprinkling preempt_{enable,disable} so liberally.

 -- Keir

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Andrew Morton]

On Fri, 16 Feb 2007 10:47:11 +0000 Keir Fraser <keir@xensource.com> wrote:

> On 16/2/07 10:09, "Andrew Morton" <akpm@linux-foundation.org> wrote:
> 
> > Are the places where the domU code references machine addresses splattered
> > all over the code?  If not, they can just be wrapped with
> > preempt_disable/preempt_enable?
> 
> The main places where machine addresses are 'visible' are any code that
> holds a pte_t,pmd_t,pud_t,pgd_t. We hide the machine-to-pseudophysical and
> pseudophysical-to-machine translations inside e.g., pte_val() and __pte()
> (i.e., constructors and extractors for page table entries). Obviously the
> users of these macros are open coded all over the place, quite apart from
> the performance cost of sprinkling preempt_{enable,disable} so liberally.

OK, you're screwed.  I agree that the process freezer is the way out of that one.

Ingo said that he's clocked the freezer at a few milliseconds.  But if it's
any higher than that it'll need to get sped up once we convert cpu hotplug
to use it.

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Keir Fraser]

On 16/2/07 10:54, "Andrew Morton" <akpm@linux-foundation.org> wrote:

> OK, you're screwed.  I agree that the process freezer is the way out of that
> one.
> 
> Ingo said that he's clocked the freezer at a few milliseconds.  But if it's
> any higher than that it'll need to get sped up once we convert cpu hotplug
> to use it.

A few milliseconds is acceptable. Presumably that is under some stress
workload? What we'd really like is to be able to gang-hotplug CPUs to
amortise the freezer overhead.

This initial patchset does not include save/restore support anyway, so in
fact it would be consistent to have CONFIG_PREEMPT configurable. I'm sure
that we are going to have some nasty bugs to fix up as a result, but we
can't fix them until we find them! Then we can convert our save/restore code
to use the freezer before submitting it for inclusion.

 -- Keir

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disableunsupported config options.

[Keir Fraser]

On 16/2/07 10:54, "Andrew Morton" <akpm@linux-foundation.org> wrote:

> OK, you're screwed.  I agree that the process freezer is the way out of that
> one.
> 
> Ingo said that he's clocked the freezer at a few milliseconds.  But if it's
> any higher than that it'll need to get sped up once we convert cpu hotplug
> to use it.

A few milliseconds is acceptable. Presumably that is under some stress
workload? What we'd really like is to be able to gang-hotplug CPUs to
amortise the freezer overhead.

This initial patchset does not include save/restore support anyway, so in
fact it would be consistent to have CONFIG_PREEMPT configurable. I'm sure
that we are going to have some nasty bugs to fix up as a result, but we
can't fix them until we find them! Then we can convert our save/restore code
to use the freezer before submitting it for inclusion.

 -- Keir