Re: [RFC v2 02/14] filemap: align the index to mapping_min_order in the page cache

Pankaj Raghav (Samsung)
2024-02-13T21:14:03Z

> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 750e779c23db..323a8e169581 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -2479,14 +2479,16 @@ static int filemap_get_pages(struct kiocb *iocb, size_t count,
> > {
> > struct file *filp = iocb->ki_filp;
> > struct address_space *mapping = filp->f_mapping;
> > + unsigned int min_nrpages = mapping_min_folio_nrpages(mapping);
> > struct file_ra_state *ra = &filp->f_ra;
> > - pgoff_t index = iocb->ki_pos >> PAGE_SHIFT;
> > + pgoff_t index = round_down(iocb->ki_pos >> PAGE_SHIFT, min_nrpages);
> > pgoff_t last_index;
> > struct folio *folio;
> > int err = 0;
> > /* "last_index" is the index of the page beyond the end of the read */
> > last_index = DIV_ROUND_UP(iocb->ki_pos + count, PAGE_SIZE);
> > + last_index = round_up(last_index, min_nrpages);
>
> Huh? 'last_index' is unset here; sure you mean 'iocb->ki_pos + count' ?
>
> And what's the rationale to replace 'DIV_ROUND_UP' with 'round_up' ?

Actually we are not replacing DIV_ROUND_UP, we are just adding another
round_up operation to min_nrpages pages after we get the last_index by
converting the bytes to page count. The main idea is to align the
last_index to min_order, if it is set. AFAIK, there is no one helper
that can do both lines in one shot.

>
> > retry:
> > if (fatal_signal_pending(current))
> > return -EINTR;