| Message ID | 20190626120333.13310-1-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] iomap: don't mark the inode dirty in iomap_write_end | expand |
On Wed, Jun 26, 2019 at 02:03:32PM +0200, Andreas Gruenbacher wrote: > Marking the inode dirty for each page copied into the page cache can be > very inefficient for file systems that use the VFS dirty inode tracking, > and is completely pointless for those that don't use the VFS dirty inode > tracking. So instead, only set an iomap flag when changing the in-core > inode size, and open code the rest of __generic_write_end. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> Nitpick: a patch from you should never have me as the first signoff. Just drop it, and if you feel fancy add a 'Partially based on code from Christoph Hellwig.' sentence. Not that I care much. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> Doesn't the series also need a third patch reducing the amount of mark_inode_dirty calls as per your initial proposal?
On Wed, 26 Jun 2019 at 14:55, Christoph Hellwig <hch@lst.de> wrote: > Doesn't the series also need a third patch reducing the amount > of mark_inode_dirty calls as per your initial proposal? The page dirtying already reduces from once per page to once per mapping, so that should be good enough. Thanks, Andreas
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c index 93ea1d529aa3..f4b895fc632d 100644 --- a/fs/gfs2/bmap.c +++ b/fs/gfs2/bmap.c @@ -1182,6 +1182,8 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, if (ip->i_qadata && ip->i_qadata->qa_qd_num) gfs2_quota_unlock(ip); + if (iomap->flags & IOMAP_F_SIZE_CHANGED) + mark_inode_dirty(inode); gfs2_write_unlock(inode); out: diff --git a/fs/iomap.c b/fs/iomap.c index 12654c2e78f8..97569064faaa 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -777,6 +777,7 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len, unsigned copied, struct page *page, struct iomap *iomap) { const struct iomap_page_ops *page_ops = iomap->page_ops; + loff_t old_size = inode->i_size; int ret; if (iomap->type == IOMAP_INLINE) { @@ -788,7 +789,19 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len, ret = __iomap_write_end(inode, pos, len, copied, page, iomap); } - __generic_write_end(inode, pos, ret, page); + /* + * Update the in-memory inode size after copying the data into the page + * cache. It's up to the file system to write the updated size to disk, + * preferably after I/O completion so that no stale data is exposed. + */ + if (pos + ret > old_size) { + i_size_write(inode, pos + ret); + iomap->flags |= IOMAP_F_SIZE_CHANGED; + } + unlock_page(page); + + if (old_size < pos) + pagecache_isize_extended(inode, old_size, pos); if (page_ops && page_ops->page_done) page_ops->page_done(inode, pos, copied, page, iomap); put_page(page); diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 2103b94cb1bf..1df9ea187a9a 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -35,6 +35,7 @@ struct vm_fault; #define IOMAP_F_NEW 0x01 /* blocks have been newly allocated */ #define IOMAP_F_DIRTY 0x02 /* uncommitted metadata */ #define IOMAP_F_BUFFER_HEAD 0x04 /* file system requires buffer heads */ +#define IOMAP_F_SIZE_CHANGED 0x08 /* file size has changed */ /* * Flags that only need to be reported for IOMAP_REPORT requests: