Re: [PATCH] memory: transaction API

Avi Kivity
2011-07-21T15:11:26Z

On 07/21/2011 06:05 PM, Jan Kiszka wrote:
> >
> > The point is _update() can only make changes for one region atomic,
> > while _commit() is more general. You can sometimes batch all changes
> > into a single container region, but sometimes it is clumsy, and
> > sometimes impossible.
> >
> > Deletion and creation are needed because we can't update an alias'
> > offset. I guess I can add that functionality. But it still isn't as
> > general as _commit().
>
> OK. What about providing _update wrappers? They could be implemented
> internally by the memory API in terms of begin - remove region - change
> region object - re-add region - end. That would avoid boilerplate code
> on the user side and still keep the option to do open-coded transaction
> also outside the core.

It's pretty easy to do updates without resorting to transactions. If
it's just some property, you simply call m_r_update_topology(). Things
like priority need a bit more logic to keep the list in sorted order,
but it's not really difficult.

> >
> >>>
> >>>> I also think now that describing a memory region offline via a struct
> >>>> and then passing that to an atomic add/del/update would be a more handy
> >>>> and future-proof API than an increasing number set functions.
> >>>
> >>> Maybe. But it's not sufficient for atomic changes involving multiple
> >>> regions.
> >>
> >> Right. The question is still if there are use cases where this matters
> >> (ie. update frequencies comparable to graphic scenarios).
> >
> > Does even cirrus update this often? I would guess cirrus usually uses
> > the linear framebuffer, no?
>
> Not sure how this mode is called, but when vram is mapped linearly into
> the 0xa0000 range via two 32K banks, you get quite a few updates on
> larger screen changes.
>

Ok. grub2 again? or another guest?

> >
> >> But such an IOCTL would resolve our problem with dropping a logged
> >> region as well, right?
> >
> > Yes, if done right. Still we need to support older kernels.
>
> We will need workarounds like we have today, e.g. confining PAM memory
> region fragmentation to certain patterns that known OSes require. Full,
> correct support would remain the privilege of host kernels that allow to
> combine multi-region updates to an atomic operation (+ returning or
> preserving dirty logs).

An atomic update + dirty log fetch can be emulated by temporarily
freezing all vcpus.

--
error compiling committee.c: too many arguments to function