On Fri, 26 Apr 2019 at 10:30, Jan Kara <[email protected]> wrote: > > On Thu 25-04-19 18:09:12, Andreas Gruenbacher wrote: > > Move the page_done callback into a separate iomap_page_ops structure and > > add a page_prepare calback to be called before a page is written to. In > > gfs2, we'll want to start a transaction in page_prepare and end it in > > page_done, and other filesystems that implement data journaling will > > require the same kind of mechanism. > > ... > > > @@ -674,9 +675,17 @@ iomap_write_begin(struct inode *inode, loff_t pos, > > unsigned len, unsigned flags, > > if (fatal_signal_pending(current)) > > return -EINTR; > > > > + if (page_ops) { > > + status = page_ops->page_prepare(inode, pos, len, iomap); > > + if (status) > > + return status; > > + } > > + > > Looks OK for now I guess, although I'm not sure if later some fs won't need > to get hold of the actual page in ->page_prepare() and then we will need to > switch to ->page_prepare() returning the page to use. But let's leave that > for a time when such fs wants to use iomap.
Alright. > > @@ -780,8 +794,8 @@ iomap_write_end(struct inode *inode, loff_t pos, > > unsigned len, > > ret = __iomap_write_end(inode, pos, len, copied, page, iomap); > > } > > > > - if (iomap->page_done) > > - iomap->page_done(inode, pos, copied, page, iomap); > > + if (page_ops) > > + page_ops->page_done(inode, pos, copied, page, iomap); > > Looking at the code now, this is actually flawed (preexisting problem): > __iomap_write_end or generic_write_end() will release the page reference > and so you cannot just pass it to ->page_done(). That is a potential > use-after-free... Ouch. I'm sending a fix. Thanks, Andreas
