On Wed, 2018-03-07 at 20:00 -0800, Matthew Wilcox wrote: > On Wed, Mar 07, 2018 at 11:32:26AM -0700, Toshi Kani wrote: > > +/** > > + * pud_free_pmd_page - clear pud entry and free pmd page > > + * > > + * Returns 1 on success and 0 on failure (pud not cleared). > > + */ > > +int pud_free_pmd_page(pud_t *pud) > > +{ > > + return pud_none(*pud); > > +} > > Wouldn't it be clearer if you returned 'bool' instead of 'int' here? I thought about it and decided to use 'int' since all other pud/pmd/pte interfaces, such as pud_none() above, use 'int'. > Also you didn't document the pud parameter, nor use the approved form > for documenting the return type, nor the calling context. So I would > have written it out like this: > > /** > * pud_free_pmd_page - Clear pud entry and free pmd page. > * @pud: Pointer to a PUD. > * > * Context: Caller should hold mmap_sem write-locked. > * Return: %true if clearing the entry succeeded. > */ Will do. Thanks! -Toshi