On Thu, Jul 15, 2021 at 02:51:05PM -0700, Darrick J. Wong wrote:
> On Thu, Jul 15, 2021 at 04:36:28AM +0100, Matthew Wilcox (Oracle) wrote:
> > +static int iomap_write_begin(struct inode *inode, loff_t pos, size_t len,
> > + unsigned flags, struct folio **foliop, struct iomap *iomap,
> > + struct iomap *srcmap)
> > {
> > const struct iomap_page_ops *page_ops = iomap->page_ops;
> > + struct folio *folio;
> > struct page *page;
> > + unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS;
> > int status = 0;
> >
> > BUG_ON(pos + len > iomap->offset + iomap->length);
> > @@ -604,30 +605,31 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
> > return status;
> > }
> >
> > - page = grab_cache_page_write_begin(inode->i_mapping, pos >> PAGE_SHIFT,
> > - AOP_FLAG_NOFS);
> > - if (!page) {
> > + folio = __filemap_get_folio(inode->i_mapping, pos >> PAGE_SHIFT, fgp,
>
> Ah, ok, so we're moving the file_get_pages flags up to iomap now.
Right, saves us having a folio equivalent of
grab_cache_page_write_begin(). And lets us get rid of AOP_FLAG_NOFS
eventually (although that really should be obsoleted by scoped
allocations, but one windmill at a time).
> > + struct page *page = folio_file_page(folio, pos / PAGE_SIZE);
>
> pos >> PAGE_SHIFT ?
mmm. We're inconsistent:
willy@pepe:~/kernel/folio$ git grep '/ PAGE_SIZE' mm/ fs/ |wc
92 720 6475
willy@pepe:~/kernel/folio$ git grep '>> PAGE_SHIFT' mm/ fs/ |wc
635 4582 39394
That said, there's a clear preference. It's just that we had a bug the
other day where somebody shifted by PAGE_SHIFT in the wrong direction ...
But again, this is your code, so I'll change to the shift.
