On Fri, 26 Apr 2019 at 10:30, Jan Kara <jack@xxxxxxx> 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
