On Fri, Dec 28, 2018 at 11:54:17AM +0100, Vlastimil Babka wrote:
...
> Let's add some CRIU guys in the loop (dunno if the right ones). We're
> discussing David's patch [1] that makes 'nh' and 'hg' flags reported in
> /proc/pid/smaps (and set by madvise) overridable by
> prctl(PR_SET_THP_DISABLE). This was sort of accidental behavior (but
> only for mappings created after the prctl call) before 4.13 commit
> 1860033237d4 ("mm: make PR_SET_THP_DISABLE immediately active").
>
> For David's userspace that commit is a regression as there are false
> positives when checking for vma's that are eligible for THP (=don't have
> the 'nh' flag in smaps) but didn't really obtain THP's. The userspace
> assumes it's due to fragmentation (=bad) and cannot know that it's due
> to the prctl(). But we fear that making prctl() affect smaps vma flags
> means that CRIU can't query them accurately anymore, and thus it's a
> regression for CRIU. Can you comment on that?
> Michal has a patch [2] that reports the prctl() status separately, but
> that doesn't help David's existing userspace. For CRIU this also won't
> help as long the smaps vma flags still silently included the prctl()
> status. Do you see some solution that would work for everybody?
>
> [1]
> https://www.ozlabs.org/~akpm/mmots/broken-out/mm-thp-always-specify-disabled-vmas-as-nh-in-smaps.patch
> [2]
> https://www.ozlabs.org/~akpm/mmots/broken-out/mm-proc-report-pr_set_thp_disable-in-proc.patch
First of all, thanks for CC'ing. Pavel and Mike should know more about
criu's part of THP. We'are using the smaps flags on later restore stage
only where we call madvise on particular vma area. But from general pov
it is fishy to hardcode nh flag into smaps report depedning on prctl flag
-- prctl and madvise are two different interfaces. Is it possible for
David to update their userspace tools instead? I know it shounds as
we're violating 'don't-break-user-space' term but with ability to
parse /proc/pid/status to figure out if thp is enabled/disabled should
not be that hard.
