On 02/20/24 at 02:36pm, Kirill A. Shutemov wrote: > On Tue, Feb 20, 2024 at 06:25:43PM +0800, Baoquan He wrote: > > > I am not sure what part of the comment you see doesn't reflect the > > > behaviour. From my PoV, changed code matches the comment closer that > > > original. > > > > Oh, I didn't make it clear. I mean update the code comment for > > lookup_address(), and add code comment for lookup_address_in_pgd() to > > mention the level thing. Maybe it's a chance to do that. > > > > ===1> > > * > > * Lookup the page table entry for a virtual address. Return a pointer > > * to the entry and the level of the mapping. > > * > > * Note: We return pud and pmd either when the entry is marked large > > ~~~~~~~~~~~ seems we return p4d too > > * or when the present bit is not set. Otherwise we would return a > > * pointer to a nonexisting mapping. > > ~~~~~~~~~~~~~~~ NULL? > > */ > > pte_t *lookup_address(unsigned long address, unsigned int *level) > > { > > return lookup_address_in_pgd(pgd_offset_k(address), address, level); > > } > > EXPORT_SYMBOL_GPL(lookup_address); > > === > > > > ===2> > > /* > > * Lookup the page table entry for a virtual address in a specific pgd. > > * Return a pointer to the entry and the level of the mapping. > > ~~ also could return NULL if it's none entry. And do we need to > > mention the level thing? > > */ > > pte_t *lookup_address_in_pgd(pgd_t *pgd, unsigned long address, > > unsigned int *level) > > ... > > } > > > > What about this fixup: Some nitpicks. > > diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c > index 3612e3167147..425ff6e192e6 100644 > --- a/arch/x86/mm/pat/set_memory.c > +++ b/arch/x86/mm/pat/set_memory.c > @@ -657,7 +657,8 @@ static inline pgprot_t verify_rwx(pgprot_t old, pgprot_t new, unsigned long star > > /* > * Lookup the page table entry for a virtual address in a specific pgd. > - * Return a pointer to the entry and the level of the mapping. > + * Return a pointer to the entry and the level of the mapping (or NULL if > + * the entry is none) and level of the entry. ^ this right parenthesis may need be moved to the end. ======= * Return a pointer to the entry and the level of the mapping (or NULL if * the entry is none and level of the entry). ======= > */ > pte_t *lookup_address_in_pgd(pgd_t *pgd, unsigned long address, > unsigned int *level) > @@ -704,9 +705,8 @@ pte_t *lookup_address_in_pgd(pgd_t *pgd, unsigned long address, > * Lookup the page table entry for a virtual address. Return a pointer > * to the entry and the level of the mapping. > * > - * Note: We return pud and pmd either when the entry is marked large > - * or when the present bit is not set. Otherwise we would return a > - * pointer to a nonexisting mapping. > + * Note: the function returns p4d, pud and pmd either when the entry is marked ~~~ ^ s/and/or/ > + * large or when the present bit is not set. Otherwise it returns NULL. > */ > pte_t *lookup_address(unsigned long address, unsigned int *level) > { > -- > Kiryl Shutsemau / Kirill A. Shutemov > _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec