Messages in this thread | | | Date | Tue, 8 Jan 2019 17:51:07 +0800 | From | Guo Ren <> | Subject | Re: Linux 5.0-rc1 (test results) |
| |
Hi Linus, On Mon, Jan 07, 2019 at 03:21:45PM -0800, Linus Torvalds wrote: > On Mon, Jan 7, 2019 at 11:26 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > Bisect points to commit 4cf58924951ef ("mm: treewide: remove unused address > > argument from pte_alloc functions"). Interesting - wasn't that supposed > > to be automatic ? > > > > csky does use the the removed address argument, so I won't even try to > > provide a patch. Copying csky maintainer instead. > > Hmm. Interesting. The csky code seems to have some odd "poison pte > contents with ones if the address has the high bit set". PTE contents is the only _PAGE_GLOBAL bit which could let mmu ignore the ASID. One tlb entry is composed of 2 pfns and there is one GLOBAL bit in the tlb entry. When C-SKY MMU hard-refill a tlb entry into the TLB buffer, it will get pfn0-GLOBAL & pfn1-GLOBAL from the memory and put the result into the TLB buffer.
If pfn0 is valid & pfn1 is invalid, we also must keep invalid pte_t with GLOBAL bit set for C-SKY MMU.
Also see pte_clear() and pte_none() in pgtable.h.
> > Which makes little or no sense. The "high bit set" case is for kernel > page tables, but that's exactly the "pte_alloc_one()" vs > "pte_alloc_one_kernel()" distinction. > > So testing the address seems entirely wrong. Yes, testing address is no necessary, I'll remove it.
> > But there's other strangeness in there too. For example, > pte_alloc_one_kernel() will just write directly to the page. And > pte_alloc_one() will do a "kmap_atomic()" on the page it allocates, > except since it uses GFP_KERNEL, that's entirely pointless. > > Is the alloc_pages() in pte_alloc_one() perhaps meant to use > GFP_HIGHUSER instead? No GFP_HIGHUSER, kmap_atomic should be removed and I'll use __GFP_ZERO instead.
> Is this perhaps some copy-paste issue? :P
> > So I *think* the removal of the 'address' use in csky should be > simple, but yes, this needs a csky maintainer to look at. > Here is my new implementation:
static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm) { pte_t *pte; unsigned long i;
pte = (pte_t *) __get_free_page(GFP_KERNEL | __GFP_RETRY_MAYFAIL); ^^^^^^^^^^^^^^^^^^^ It's necessary ? x86 & arm don't use it. if (!pte) return NULL;
for (i = 0; i < PAGE_SIZE/sizeof(pte_t); i++) (pte + i)->pte_low = _PAGE_GLOBAL;
return pte; }
static inline struct page *pte_alloc_one(struct mm_struct *mm) { struct page *pte;
pte = alloc_pages(GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_ZERO, 0); if (!pte) return NULL;
if (!pgtable_page_ctor(pte)) { __free_page(pte); return NULL; }
return pte; }
Best Regards Guo Ren
| |