Re: A mapcount riddle

[Michal Hocko <mhocko@suse.com>]

On Thu 26-01-23 09:51:12, Mike Kravetz wrote:
> On 01/26/23 10:16, Michal Hocko wrote:
> > On Wed 25-01-23 09:59:15, Mike Kravetz wrote:
> > > On 01/25/23 09:24, Michal Hocko wrote:
> > > > On Tue 24-01-23 12:56:24, Mike Kravetz wrote:
> > > > > At first thought this seems bad.  However, I believe this has been the
> > > > > behavior since hugetlb PMD sharing was introduced in 2006 and I am
> > > > > unaware of any reported issues.  I did a audit of code looking at
> > > > > mapcount.  In addition to the above issue with smaps, there appears
> > > > > to be an issue with 'migrate_pages' where shared pages could be migrated
> > > > > without appropriate privilege.
> > > > > 
> > > > > 	/* With MPOL_MF_MOVE, we migrate only unshared hugepage. */
> > > > > 	if (flags & (MPOL_MF_MOVE_ALL) ||
> > > > > 	    (flags & MPOL_MF_MOVE && page_mapcount(page) == 1)) {
> > > > > 		if (isolate_hugetlb(page, qp->pagelist) &&
> > > > > 			(flags & MPOL_MF_STRICT))
> > > > > 			/*
> > > > > 			 * Failed to isolate page but allow migrating pages
> > > > > 			 * which have been queued.
> > > > > 			 */
> > > > > 			ret = 1;
> > > > > 	}
> > > > 
> > > > Could you elaborate what is problematic about that? The whole pmd
> > > > sharing is a cooperative thing. So if some of the processes decides to
> > > > migrate the page then why that should be a problem for others sharing
> > > > that page via page table? Am I missing something obvious?
> > > 
> > > Nothing obvious.  It is just that the semantics seem to be that you can
> > > only move shared pages if you have CAP_SYS_NICE.
> > 
> > Correct
> > 
> > > Certainly cooperation
> > > is implied for shared PMDs, but I would guess that most applications are
> > > not even aware they are sharing PMDs.
> > 
> > How come? They have to explicitly map those hugetlb pages to the same
> > address. Or is it common that the mapping just lands there by accident?
> 
> Mapping to the same address is not required for PMD sharing.  What is
> required is that the alignment of PUD_SIZE offsets within the mapped object
> (file) are mapped to PUD_SIZE aligned virtual addresses.  That may not be
> clear as it is difficult to describe.  Bottom like is that addresses do not
> need to match.

Hmm, my bad then. I thought that is a strict requirement. But looking at
the code page_table_shareable talks about pmd_index indeed. I must have
misremember.

I do agree that it is much simpler to hit into page table sharing for
large mappings unintentionally - especially if they are GB aligned which
is not really that unexpected.
-- 
Michal Hocko
SUSE Labs