On Tue, Feb 27, 2024 at 11:22:24AM -0500, Kent Overstreet wrote:
> On Tue, Feb 27, 2024 at 11:06:37AM +0100, Pankaj Raghav (Samsung) wrote:
> > On Mon, Feb 26, 2024 at 02:40:42PM +0000, Matthew Wilcox wrote:
> > > On Mon, Feb 26, 2024 at 10:49:26AM +0100, Pankaj Raghav (Samsung) wrote:
> > > > From: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> > > >
> > > > Supporting mapping_min_order implies that we guarantee each folio in the
> > > > page cache has at least an order of mapping_min_order. So when adding new
> > > > folios to the page cache we must ensure the index used is aligned to the
> > > > mapping_min_order as the page cache requires the index to be aligned to
> > > > the order of the folio.
> > >
> > > This seems like a remarkably complicated way of achieving:
> > >
> > > diff --git a/mm/filemap.c b/mm/filemap.c
> > > index 5603ced05fb7..36105dad4440 100644
> > > --- a/mm/filemap.c
> > > +++ b/mm/filemap.c
> > > @@ -2427,9 +2427,11 @@ static int filemap_update_page(struct kiocb *iocb,
> > > }
> > >
> > > static int filemap_create_folio(struct file *file,
> > > - struct address_space *mapping, pgoff_t index,
> > > + struct address_space *mapping, loff_t pos,
> > > struct folio_batch *fbatch)
> > > {
> > > + pgoff_t index;
> > > + unsigned int min_order;
> > > struct folio *folio;
> > > int error;
> > >
> > > @@ -2451,6 +2453,8 @@ static int filemap_create_folio(struct file *file,
> > > * well to keep locking rules simple.
> > > */
> > > filemap_invalidate_lock_shared(mapping);
> > > + min_order = mapping_min_folio_order(mapping);
> > > + index = (pos >> (min_order + PAGE_SHIFT)) << min_order;
> >
> > That is some cool mathfu. I will add a comment here as it might not be
> > that obvious to some people (i.e me).
>
> you guys are both wrong, just use rounddown()
Umm, what do you mean just use rounddown? rounddown to ...?
We need to get index that are in PAGE units but aligned to min_order
pages.
The original patch did this:
index = mapping_align_start_index(mapping, iocb->ki_pos >> PAGE_SHIFT);
Which is essentially a rounddown operation (probably this is what you
are suggesting?).
So what willy is proposing will do the same. To me, what I proposed is
less complicated but to willy it is the other way around.
