Re: [PATCH v2 20/28] iomap: Convert iomap_write_begin() and iomap_write_end() to folios

[Darrick J. Wong]

On Wed, Nov 17, 2021 at 02:31:26PM +0000, Matthew Wilcox wrote:
> On Tue, Nov 16, 2021 at 08:31:27PM -0800, Darrick J. Wong wrote:
> > > @@ -764,16 +761,17 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
> > >  			break;
> > >  		}
> > >  
> > > -		status = iomap_write_begin(iter, pos, bytes, &page);
> > > +		status = iomap_write_begin(iter, pos, bytes, &folio);
> > >  		if (unlikely(status))
> > >  			break;
> > >  
> > > +		page = folio_file_page(folio, pos >> PAGE_SHIFT);
> > >  		if (mapping_writably_mapped(iter->inode->i_mapping))
> > >  			flush_dcache_page(page);
> > >  
> > >  		copied = copy_page_from_iter_atomic(page, offset, bytes, i);
> > 
> > Hrmm.  In principle (or I guess even a subsequent patch), if we had
> > multi-page folios, could we simply loop the pages in the folio instead
> > of doing a single page and then calling back into iomap_write_begin to
> > get (probably) the same folio?
> > 
> > This looks like a fairly straightforward conversion, but I was wondering
> > about that one little point...
> 
> Theoretically, yes, we should be able to do that.  But all of this code
> is pretty subtle ("What if we hit a page fault?  What if we're writing
> to part of this folio from an mmap of a different part of this folio?
> What if it's !Uptodate?  What if we hit this weird ARM super-mprotect
> memory tag thing?  What if ...") and, frankly, I got scared.  So I've
> left that as future work; someone else can try to wrap their brain around
> all of this.

<nod> That's roughly the same conclusion I came to -- conceptually we
could keep walking pages until we hit /any/ problem or other difference
with the first page that we don't feel like dealing with, and pass that
count to iomap_end... but no need to try that right this second.

Just checking that I grokked what's going on in this series. :)

--D