From: Andreas Gruenbacher <agruenba@redhat.com> To: Christoph Hellwig <hch@lst.de>, linux-fsdevel@vger.kernel.org Cc: cluster-devel@redhat.com, linux-xfs@vger.kernel.org, Andreas Gruenbacher <agruenba@redhat.com> Subject: [PATCH v2 1/3] iomap: don't mark the inode dirty in iomap_write_end Date: Wed, 26 Jun 2019 15:23:33 +0200 [thread overview] Message-ID: <20190626132335.14809-1-agruenba@redhat.com> (raw) 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> --- 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
WARNING: multiple messages have this Message-ID (diff)
From: Andreas Gruenbacher <agruenba@redhat.com> To: cluster-devel.redhat.com Subject: [Cluster-devel] [PATCH v2 1/3] iomap: don't mark the inode dirty in iomap_write_end Date: Wed, 26 Jun 2019 15:23:33 +0200 [thread overview] Message-ID: <20190626132335.14809-1-agruenba@redhat.com> (raw) 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> --- 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
next reply other threads:[~2019-06-26 13:23 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-26 13:23 Andreas Gruenbacher [this message] 2019-06-26 13:23 ` [Cluster-devel] [PATCH v2 1/3] iomap: don't mark the inode dirty in iomap_write_end Andreas Gruenbacher 2019-06-26 13:23 ` [PATCH v2 2/3] fs: fold __generic_write_end back into generic_write_end Andreas Gruenbacher 2019-06-26 13:23 ` [Cluster-devel] " Andreas Gruenbacher 2019-06-26 20:09 ` Darrick J. Wong 2019-06-26 20:09 ` [Cluster-devel] " Darrick J. Wong 2019-06-26 13:23 ` [PATCH v2 3/3] iomap: fix page_done callback for short writes Andreas Gruenbacher 2019-06-26 13:23 ` [Cluster-devel] " Andreas Gruenbacher 2019-06-26 13:37 ` Christoph Hellwig 2019-06-26 13:37 ` [Cluster-devel] " Christoph Hellwig 2019-06-26 20:09 ` Darrick J. Wong 2019-06-26 20:09 ` [Cluster-devel] " Darrick J. Wong 2019-06-26 20:09 ` [PATCH v2 1/3] iomap: don't mark the inode dirty in iomap_write_end Darrick J. Wong 2019-06-26 20:09 ` [Cluster-devel] " Darrick J. Wong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190626132335.14809-1-agruenba@redhat.com \ --to=agruenba@redhat.com \ --cc=cluster-devel@redhat.com \ --cc=hch@lst.de \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.