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.
> @@ -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...
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
