Re: [Qemu-devel] [PATCH] memory: Reintroduce dirty flag to optimize changes on disabled regions

[Avi Kivity]

On 11/05/2012 10:51 AM, Jan Kiszka wrote:
> On 2012-11-05 09:12, Avi Kivity wrote:
> > On 11/05/2012 08:26 AM, Jan Kiszka wrote:
> >> On 2012-11-04 20:21, Avi Kivity wrote:
> >>> On 11/04/2012 10:30 AM, Jan Kiszka wrote:
> >>>> From: Jan Kiszka <address@hidden>
> >>>>
> >>>> Cirrus is triggering this, e.g. during Win2k boot: Changes only on
> >>>> disabled regions require no topology update when transaction depth drops
> >>>> to 0 again.
> >>>
> >>> 817dcc5368988b0 (pci: give each device its own address space) mad this
> >>> much worse by multiplying the number of address spaces.  Each change is
> >>> now evaluated N+2 times, where N is the number of PCI devices.  It also
> >>> causes a corresponding expansion in memory usage.
> >>
> >> I know... But this regression predates your changes, is already visible
> >> right after 02e2b95fb4.
> >>
> >>>
> >>> I want to address this by caching AddressSpaceDispatch trees with the
> >>> key being the contents of the FlatView for that address space.  This
> >>> will drop the number of distinct trees to 2-4 (3 if some devices have
> >>> PCI_COMMAND_MASTER disabled, 4 if the PCI address space is different
> >>> from the cpu memory address space) but will fail if we make each address
> >>> space different (for example filtering out the device's own BARs).
> >>>
> >>> If this change also improves cpu usage sufficiently, then it will be
> >>> better than your patch, which doesn't recognize changes in an enabled
> >>> region inside a disabled or hidden region.
> >>
> >> True, though the question is how common such scenarios are. This one
> >> (cirrus with win2k) is already special.
> >>
> >>>  In other words, your patch
> >>> fits the problem at hand but isn't general.  On the other hand my
> >>> approach doesn't eliminate render_memory_region(), just the exec.c stuff
> >>> and listener updates.  So we need to understand where the slowness comes
> >>> from.
> >>
> >> I would just like to have some even intermediate solution for 1.3. We
> >> can still make it more perfect later on if required.
> >>
> > 
> > I think we should apply a v2 then, the more general optimizations will
> > take some time.
>
> OK - what should v2 do differently?
>

As I noted, init and destroy cannot cause a topology update.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.