On Wed 26-07-17 13:11:46, Punit Agrawal wrote: > Hi Michal, > > Michal Hocko <mhocko@xxxxxxxxxx> writes: > > > On Wed 26-07-17 10:50:38, Michal Hocko wrote: > >> On Tue 25-07-17 16:41:14, Punit Agrawal wrote: > >> > When walking the page tables to resolve an address that points to > >> > !p*d_present() entry, huge_pte_offset() returns inconsistent values > >> > depending on the level of page table (PUD or PMD). > >> > > >> > It returns NULL in the case of a PUD entry while in the case of a PMD > >> > entry, it returns a pointer to the page table entry. > >> > > >> > A similar inconsitency exists when handling swap entries - returns NULL > >> > for a PUD entry while a pointer to the pte_t is retured for the PMD > >> > entry. > >> > > >> > Update huge_pte_offset() to make the behaviour consistent - return NULL > >> > in the case of p*d_none() and a pointer to the pte_t for hugepage or > >> > swap entries. > >> > > >> > Document the behaviour to clarify the expected behaviour of this > >> > function. This is to set clear semantics for architecture specific > >> > implementations of huge_pte_offset(). > >> > >> hugetlb pte semantic is a disaster and I agree it could see some > >> cleanup/clarifications but I am quite nervous to see a patchi like this. > >> How do we check that nothing will get silently broken by this change? > > Glad I'm not the only one who finds the hugetlb semantics somewhat > confusing. :) This is a huge understatement. It is a source of nightmares. > I've been running tests from mce-test suite and libhugetlbfs for similar > changes we did on arm64. There could be assumptions that were not > exercised but I'm not sure how to check for all the possible usages. > > Do you have any other suggestions that can help improve confidence in > the patch? Unfortunatelly I don't. I just know there were many subtle assumptions all over the place so I am rather careful to not touch the code unless really necessary. That being said, I am not opposing your patch. -- Michal Hocko SUSE Labs
