Re: [Xen-devel] Regression: x86/mm: new _PTE_SWP_SOFT_DIRTY bit conflicts with existing use

[Cyrill Gorcunov <gorcunov@xxxxxxxxx>]

On Thu, Aug 22, 2013 at 07:56:26AM +0100, Jan Beulich wrote:
> >>> On 21.08.13 at 18:19, Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote:
> > On Wed, Aug 21, 2013 at 05:03:13PM +0100, Jan Beulich wrote:
> >> > 
> >> > Only to non-present ptes, as far as I know.
> >> 
> >> That's not really any guarantee. And the accessor functions also
> >> don't check that they'd be used on non-present PTEs only.
> > 
> > Wait. This _PAGE_SWP_SOFT_DIRTY bit (which is in real PSE bit) assigned
> > in only one place -- in try_to_unmap_one(). The PTE get non-present then
> > and consists of swap entry format. I don't see any accessor to such entry
> > without testing if it's swap entry or pte-none. What I'm missing?
> 
> Fact is that this
> 
> static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
> {
>       return pte_set_flags(pte, _PAGE_SWP_SOFT_DIRTY);
> }
> 
> has no checking whatsoever that the PTE being modified is a
> non-present one, not even in any of the debugging modes. It
> would be a different thing if the above acted on a swp_entry_t.
> 
> The fact that there currently may be just a single call site (where
> the caller guarantees the non-present state) is no guarantee that
> in the future another one won't appear, and then result in very
> hard to debug problems.

Ok, how about this?

static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
{
        BUG_ON(pte_present(pte));
        return pte_set_flags(pte, _PAGE_SWP_SOFT_DIRTY);
}

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