Re: [Xen-devel] [PATCH for-4.9] x86/mm: Mark pages as dirty after (rather than before) writing to them

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.05.17 at 19:08, <andrew.cooper3@xxxxxxxxxx> wrote:
> Otherwise a logdirty client can race with observing the page becoming dirty,
> and copy the frame before the write is complete and end up with a stale
> version.

So I must have looked at the wrong examples back when writing this
code. There are a couple of oddities, but in particular I wonder why
you wouldn't want to also move the use in do_update_descriptor().
Similarly I'd expect hvm_unmap_guest_frame() would want to mark
the page dirty in the !permanent case instead of
_hvm_map_guest_frame() (I'm afraid it's again me who is guilty of
having got this one wrong).

As to oddities:

Why does alloc_page_type() need to mark a page dirty? Wouldn't
doing so in free_page_type() suffice?

Why does free_page_type() have an extra paging_mode_enabled()
check which no other callers have?

Why are pages being marked dirty when they are being pinned? As
per above, doing this when unpinning ought to suffice, and both
seem redundant with what alloc_page_type() / free_page_type()
do respectively.

Why does MMU_MACHPHYS_UPDATE require dirtying the MFN?
Same for p2m_pod_demand_populate() and
__mem_sharing_unshare_page() doing this next to a
set_gpfn_from_mfn() call. (There I additionally wonder whether
it shouldn't honor global log-dirty mode instead of unconditionally
setting p2m_ram_rw.)

Why does hvm_add_ioreq_gmfn() mark the page dirty, but
hvm_remove_ioreq_gmfn() doesn't (despite calling clear_page())?
Isn't the page part of the guest no matter that it's not part of its
physmap?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel