Re: [PATCH v2] After swapout/swapin private dirty mappings are reported clean in smaps

[Hugh Dickins <hugh.dickins@tiscali.co.uk>]

On Wed, Sep 15, 2010 at 10:24 AM, Matt Mackall <mpm@selenic.com> wrote:

> But that's my point: the consistency problem is NOT in smaps. The page
> is NOT marked dirty, ergo smaps doesn't report it as dirty. Whether or
> not there is MORE information smaps could be reporting is irrelevant,
> the information it IS reporting is consistent with the underlying VM
> data. If there's an inconsistency about what it means to be clean, it's
> either in the VM or in your head.
>
> And I frankly think it's in the VM.

I don't believe there's any problem in the VM here, we'd be having
SIGSEGVs all over if there were.

The problem is that /proc/pid/smaps exports a simplified view of the
VM, and Richard and Nikanth were hoping that it gave them some info
which it has never pretended to give them,

It happens to use a pte_dirty(ptent) test: you could argue that that
should be pte_dirty(ptent) || PageDirty(page) (which would then "fix
the issue" which Richard sees with swapoff/swapon), or you could argue
that it should be pte_dirty(ptent) || PageDirty(page) ||
PageSwapCache(page) (which would then note clean copies of swap cache
as dirty in the sense which Richard and Nikanth are interested in).

But after these years, we should probably assume that most users of
/proc/pid/smaps are used to the existing pte_dirty(ptent) test, and
would be troubled by a departure from it.

>
> In any case, I don't think Nikanth's fix is the right fix, as it
> basically says "you can't trust any of this". Either swap should return
> the pages to their pre-swap dirty state in the VM, or we should add
> another field here:
>
> Weird_Anon_Page_You_Should_Pretend_Is_Private_Dirty: 8 kB

I think that the most widely useful but simple extension of
/proc/pid/smaps, that would give them the info they want, would indeed
be to counts ptes pointing to PageAnon pages and report that total on
an additional line (say, just before "Swap:"); but there's no need for
the derogatory name you propose there, "Anon:" would suit fine!

The original idea of testing for vma->anon_vma was a good one, but it
doesn't fit so well into the /proc/pid/smaps layout; and showing the
actual count of Anon pages could be useful to others (though generally
it would be Anon+Swap they'd be interested in).

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>