| Message ID | 20190626132335.14809-1-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] iomap: don't mark the inode dirty in iomap_write_end | expand |
On Wed, Jun 26, 2019 at 03:23:33PM +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. > > Partially based on code from Christoph Hellwig. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/gfs2/bmap.c | 2 ++ > fs/iomap.c | 15 ++++++++++++++- > include/linux/iomap.h | 1 + > 3 files changed, 17 insertions(+), 1 deletion(-) > > 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: > -- > 2.20.1 >
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: