> On May 15, 2021, at 2:14 PM, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Sat, May 15, 2021 at 10:55:19AM +0000, William Kucharski wrote: >>> +/** >>> + * folio_page - Return a page from a folio. >>> + * @folio: The folio. >>> + * @n: The page number to return. >>> + * >>> + * @n is relative to the start of the folio. It should be between >>> + * 0 and folio_nr_pages(@folio) - 1, but this is not checked for. >> >> Please add a statement noting WHY @n isn't checked since you state it >> should be. Something like "...but this is not checked for because this is >> a hot path." > > Hmm ... how about this: > > /** > * folio_page - Return a page from a folio. > * @folio: The folio. > * @n: The page number to return. > * > * @n is relative to the start of the folio. This function does not > * check that the page number lies within @folio; the caller is presumed > * to have a reference to the page. > */ > #define folio_page(folio, n) nth_page(&(folio)->page, n) > > It occurred to me that it is actually useful (under some circumstances) > for referring to a page outside the base folio. For example when > dealing with bios that have merged consecutive pages together into a > single bvec (ok, bios don't use folios, but it would be reasonable if > they did in future). I like that comment better, or you could just state bounds checking of the returned page number is left to the caller; that would cover both the normal case and possible future usage for calculations outside the base folio.