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

Hugh Dickins
2010-09-15T21:47:44Z

On Wed, Sep 15, 2010 at 12:46 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Wed, 2010-09-15 at 12:18 -0700, Hugh Dickins wrote:
>> 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),
>
> That might be interesting. Are there any other notable cases where
> pte_dirty() differs from PageDirty()?

I don't know about "other notable". A page may very well be PageDirty
(e.g. modified by a write system call) without any of the ptes
pointing to it (if there even are any) marked as pte_dirty. A page
may very well not be marked PageDirty yet, though one or more of the
ptes pointing to it have been marked pte_dirty when userspace made a
write access to the page via that pte. Traditionally (when
/proc/pid/smaps was first reporting dirty versus clean ptes) the pte
dirtiness would later be found and propagated through to PageDirtiness
(clearing the pte dirtiness), which would later be cleaned when the
page was written out to backing store (file or swap).

PeterZ's writeback work in 2.6.19 (set_page_dirty_balance,
clear_page_dirty_for_io etc.) tightened up writeback from (most: tmpfs
is one exception) shared file mmaps, synchronizing the PageDirty more
carefully with the pte_dirty; and perhaps there is some inconsistency
there, that we never felt compelled to keep pte and Page so tightly in
synch in the anonymous/Swap case - it would have been unnecessary
overhead (though I repeatedly forget the essence of why not - file
syncing, and pdflush activity, were relevant considerations; but I
cannot now put my finger on precisely why shared file writing needed
to be fixed, but anonymous dirtying could be left unchanged).

But even if you replace smaps's pte_dirty(ptent) tests by
pte_dirty(ptent) || PageDirty(page) tests, it wouldn't be doing what
Richard and Nikanth want - they want clean ptes of clean PageSwapCache
to be reported as dirty, despite being clean copies of backing store;
and I can understand your reluctance to go that far. I think
reporting "Anon:" pages is more useful - in part because we have no
counts of Anon+Swap, yet that's the quantity which vm_enough_memory
may place a limit upon (but beware, it does of course get more
complicated: tmpfs files come out of that tally too, but would not
show up in this way).

Hugh