[PATCH 00/10] gfs2 iomap buffered write support

[PATCH 00/10] gfs2 iomap buffered write support

[Andreas Gruenbacher]

Hello,

this patch queue converts gfs2 to use iomap for buffered writes, which
uses multi-page block allocations for large writes instead of requiring
a separate allocation for each page of data.

The patches apply on top of the gfs2 punch-hole patch queue [*].

So far, the only user of iomap_file_buffered_write was xfs, which
doesn't do data journaling.  To support gfs2's data journaling, patch
08/10 adds a new iomap_written iomap operation: if defined, this
operation is called by iomap_file_buffered_write whenever a page has
been written to.

This patch queue doesn't convert direct I/O, so we still have a
remaining user of the old gfs2_write_begin / gfs2_write_end interface
left.  Once direct I/O has been converted to use iomap, we'll get rid of
that code, though.

Thanks,
Andreas

[*] https://www.redhat.com/archives/cluster-devel/2017-December/msg00089.html

Andreas Gruenbacher (10):
  gfs2: Typo fixes
  gfs2: Add gfs2_max_stuffed_size
  gfs2: Minor gfs2_page_add_databufs cleanup
  gfs2: gfs2_stuffed_write_end cleanup
  gfs2: gfs2_stuffed_write_end cleanup (fixup)
  gfs2: Remove ordered write mode handling from gfs2_trans_add_data
  gfs2: Iomap cleanups and improvements
  iomap: New iomap_written operation
  gfs2: Implement iomap buffered write support (1)
  gfs2: Implement iomap buffered write support (2)

 fs/gfs2/aops.c        | 178 ++++++++++++++++------
 fs/gfs2/aops.h        |  19 +++
 fs/gfs2/bmap.c        | 407 +++++++++++++++++++++++++++++++++++++-------------
 fs/gfs2/bmap.h        |   5 +-
 fs/gfs2/dir.c         |   3 +-
 fs/gfs2/file.c        |  49 +++++-
 fs/gfs2/incore.h      |   5 +
 fs/gfs2/inode.c       |  10 +-
 fs/gfs2/log.h         |   7 +-
 fs/gfs2/quota.c       |   5 +-
 fs/gfs2/trans.c       |  27 +---
 fs/iomap.c            |  21 ++-
 include/linux/iomap.h |   9 ++
 13 files changed, 558 insertions(+), 187 deletions(-)
 create mode 100644 fs/gfs2/aops.h

-- 
2.14.3

[PATCH 01/10] gfs2: Typo fixes

[Andreas Gruenbacher]

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 1daf15a1f00c..a270effcbbfc 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -501,10 +501,9 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
  * @file: The file to read a page for
  * @page: The page to read
  *
- * This is the core of gfs2's readpage. Its used by the internal file
- * reading code as in that case we already hold the glock. Also its
+ * This is the core of gfs2's readpage. It's used by the internal file
+ * reading code as in that case we already hold the glock. Also it's
  * called by gfs2_readpage() once the required lock has been granted.
- *
  */
 
 static int __gfs2_readpage(void *file, struct page *page)
-- 
2.14.3

[PATCH 02/10] gfs2: Add gfs2_max_stuffed_size

[Andreas Gruenbacher]

Add a small inline function for computing the maximum size of a stuffed
inode instead of open coding that in several places throughout the code.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c   |  9 +++++----
 fs/gfs2/bmap.c   | 10 ++++------
 fs/gfs2/dir.c    |  3 +--
 fs/gfs2/incore.h |  5 +++++
 fs/gfs2/inode.c  |  6 ++----
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index a270effcbbfc..ec224d578bd3 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -483,8 +483,8 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
 		return error;
 
 	kaddr = kmap_atomic(page);
-	if (dsize > (dibh->b_size - sizeof(struct gfs2_dinode)))
-		dsize = (dibh->b_size - sizeof(struct gfs2_dinode));
+	if (dsize > gfs2_max_stuffed_size(ip))
+		dsize = gfs2_max_stuffed_size(ip);
 	memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
 	memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
 	kunmap_atomic(kaddr);
@@ -724,7 +724,7 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
 
 	if (gfs2_is_stuffed(ip)) {
 		error = 0;
-		if (pos + len > sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode)) {
+		if (pos + len > gfs2_max_stuffed_size(ip)) {
 			error = gfs2_unstuff_dinode(ip, page);
 			if (error == 0)
 				goto prepare_write;
@@ -831,7 +831,8 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
 	void *kaddr;
 	unsigned char *buf = dibh->b_data + sizeof(struct gfs2_dinode);
 
-	BUG_ON((pos + len) > (dibh->b_size - sizeof(struct gfs2_dinode)));
+	BUG_ON(pos + len > gfs2_max_stuffed_size(ip));
+
 	kaddr = kmap_atomic(page);
 	memcpy(buf + pos, kaddr + pos, copied);
 	flush_dcache_page(page);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 44a24d1feff4..dded1818bd2d 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -69,8 +69,8 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
 		void *kaddr = kmap(page);
 		u64 dsize = i_size_read(inode);
  
-		if (dsize > (dibh->b_size - sizeof(struct gfs2_dinode)))
-			dsize = dibh->b_size - sizeof(struct gfs2_dinode);
+		if (dsize > gfs2_max_stuffed_size(ip))
+			dsize = gfs2_max_stuffed_size(ip);
 
 		memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
 		memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
@@ -1691,8 +1691,7 @@ static int do_grow(struct inode *inode, u64 size)
 	int error;
 	int unstuff = 0;
 
-	if (gfs2_is_stuffed(ip) &&
-	    (size > (sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode)))) {
+	if (gfs2_is_stuffed(ip) && size > gfs2_max_stuffed_size(ip)) {
 		error = gfs2_quota_lock_check(ip, &ap);
 		if (error)
 			return error;
@@ -1927,8 +1926,7 @@ int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset,
 		return 0;
 
 	if (gfs2_is_stuffed(ip)) {
-		if (offset + len >
-		    sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode))
+		if (offset + len > gfs2_max_stuffed_size(ip))
 			return 1;
 		return 0;
 	}
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 06a0d1947c77..7c21aea0266b 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -170,8 +170,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
 	if (!size)
 		return 0;
 
-	if (gfs2_is_stuffed(ip) &&
-	    offset + size <= sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode))
+	if (gfs2_is_stuffed(ip) && offset + size <= gfs2_max_stuffed_size(ip))
 		return gfs2_dir_write_stuffed(ip, buf, (unsigned int)offset,
 					      size);
 
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 6e18e9793ec4..9d4d7367175f 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -861,5 +861,10 @@ static inline void gfs2_sbstats_inc(const struct gfs2_glock *gl, int which)
 
 extern struct gfs2_rgrpd *gfs2_glock2rgrp(struct gfs2_glock *gl);
 
+static inline unsigned gfs2_max_stuffed_size(const struct gfs2_inode *ip)
+{
+	return GFS2_SB(&ip->i_inode)->sd_sb.sb_bsize - sizeof(struct gfs2_dinode);
+}
+
 #endif /* __INCORE_DOT_H__ */
 
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 4e971b1c7f92..20281992d456 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1184,11 +1184,10 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry)
 static int gfs2_symlink(struct inode *dir, struct dentry *dentry,
 			const char *symname)
 {
-	struct gfs2_sbd *sdp = GFS2_SB(dir);
 	unsigned int size;
 
 	size = strlen(symname);
-	if (size > sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode) - 1)
+	if (size >= gfs2_max_stuffed_size(GFS2_I(dir)))
 		return -ENAMETOOLONG;
 
 	return gfs2_create_inode(dir, dentry, NULL, S_IFLNK | S_IRWXUGO, 0, symname, size, 0, NULL);
@@ -1205,8 +1204,7 @@ static int gfs2_symlink(struct inode *dir, struct dentry *dentry,
 
 static int gfs2_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 {
-	struct gfs2_sbd *sdp = GFS2_SB(dir);
-	unsigned dsize = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode);
+	unsigned dsize = gfs2_max_stuffed_size(GFS2_I(dir));
 	return gfs2_create_inode(dir, dentry, NULL, S_IFDIR | mode, 0, NULL, dsize, 0, NULL);
 }
 
-- 
2.14.3

[PATCH 03/10] gfs2: Minor gfs2_page_add_databufs cleanup

[Andreas Gruenbacher]

The to parameter of gfs2_page_add_databufs is passed inconsistently:
once as from + len, once as from + len - 1.  Just pass len instead.

In addition, once we're past the end, we can immediately break out of
the loop.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index ec224d578bd3..81dc28a0aba1 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -39,18 +39,21 @@
 
 
 static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
-				   unsigned int from, unsigned int to)
+				   unsigned int from, unsigned int len)
 {
 	struct buffer_head *head = page_buffers(page);
 	unsigned int bsize = head->b_size;
 	struct buffer_head *bh;
+	unsigned int to = from + len;
 	unsigned int start, end;
 
 	for (bh = head, start = 0; bh != head || !start;
 	     bh = bh->b_this_page, start = end) {
 		end = start + bsize;
-		if (end <= from || start >= to)
+		if (end <= from)
 			continue;
+		if (start >= to)
+			break;
 		if (gfs2_is_jdata(ip))
 			set_buffer_uptodate(bh);
 		gfs2_trans_add_data(ip->i_gl, bh);
@@ -189,7 +192,7 @@ static int __gfs2_jdata_writepage(struct page *page, struct writeback_control *w
 			create_empty_buffers(page, inode->i_sb->s_blocksize,
 					     BIT(BH_Dirty)|BIT(BH_Uptodate));
 		}
-		gfs2_page_add_databufs(ip, page, 0, sdp->sd_vfs->s_blocksize-1);
+		gfs2_page_add_databufs(ip, page, 0, sdp->sd_vfs->s_blocksize);
 	}
 	return gfs2_write_full_page(page, gfs2_get_block_noalloc, wbc);
 }
@@ -890,8 +893,6 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
 	struct buffer_head *dibh;
-	unsigned int from = pos & (PAGE_SIZE - 1);
-	unsigned int to = from + len;
 	int ret;
 	struct gfs2_trans *tr = current->journal_info;
 	BUG_ON(!tr);
@@ -909,7 +910,7 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
 		return gfs2_stuffed_write_end(inode, dibh, pos, len, copied, page);
 
 	if (!gfs2_is_writeback(ip))
-		gfs2_page_add_databufs(ip, page, from, to);
+		gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
 
 	ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
 	if (tr->tr_num_buf_new)
-- 
2.14.3

[PATCH 04/10] gfs2: gfs2_stuffed_write_end cleanup

[Andreas Gruenbacher]

First, change the sanity check in gfs2_stuffed_write_end to check for
the actual write size instead of the requested write size.

Second, use the existing teardown code in gfs2_write_end instead of
duplicating it in gfs2_stuffed_write_end.

Third, make the page argument optional.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c | 65 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 81dc28a0aba1..aa2031ff82c8 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -814,9 +814,8 @@ static void adjust_fs_space(struct inode *inode)
  * @inode: The inode
  * @dibh: The buffer_head containing the on-disk inode
  * @pos: The file position
- * @len: The length of the write
  * @copied: How much was actually copied by the VFS
- * @page: The page
+ * @page: The (optional) page
  *
  * This copies the data from the page into the inode block after
  * the inode data structure itself.
@@ -824,17 +823,23 @@ static void adjust_fs_space(struct inode *inode)
  * Returns: errno
  */
 static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
-				  loff_t pos, unsigned len, unsigned copied,
+				  loff_t pos, unsigned copied,
 				  struct page *page)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
-	struct gfs2_sbd *sdp = GFS2_SB(inode);
-	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
 	u64 to = pos + copied;
 	void *kaddr;
 	unsigned char *buf = dibh->b_data + sizeof(struct gfs2_dinode);
+	bool page_grabbed = false;
 
-	BUG_ON(pos + len > gfs2_max_stuffed_size(ip));
+	BUG_ON(pos + copied > gfs2_max_stuffed_size(ip));
+
+	if (!page) {
+		page = find_lock_page(inode->i_mapping, 0);
+		if (WARN_ON(!page))
+			return -ENOMEM;
+		page_grabbed = true;
+	}
 
 	kaddr = kmap_atomic(page);
 	memcpy(buf + pos, kaddr + pos, copied);
@@ -842,28 +847,17 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
 	kunmap_atomic(kaddr);
 
 	WARN_ON(!PageUptodate(page));
-	unlock_page(page);
-	put_page(page);
+
+	if (page_grabbed) {
+		unlock_page(page);
+		put_page(page);
+	}
 
 	if (copied) {
 		if (inode->i_size < to)
 			i_size_write(inode, to);
 		mark_inode_dirty(inode);
 	}
-
-	if (inode == sdp->sd_rindex) {
-		adjust_fs_space(inode);
-		sdp->sd_rindex_uptodate = 0;
-	}
-
-	brelse(dibh);
-	gfs2_trans_end(sdp);
-	if (inode == sdp->sd_rindex) {
-		gfs2_glock_dq(&m_ip->i_gh);
-		gfs2_holder_uninit(&m_ip->i_gh);
-	}
-	gfs2_glock_dq(&ip->i_gh);
-	gfs2_holder_uninit(&ip->i_gh);
 	return copied;
 }
 
@@ -877,9 +871,8 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
  * @page: The page that has been written
  * @fsdata: The fsdata (unused in GFS2)
  *
- * The main write_end function for GFS2. We have a separate one for
- * stuffed files as they are slightly different, otherwise we just
- * put our locking around the VFS provided functions.
+ * The main write_end function for GFS2. We just put our locking around the VFS
+ * provided functions.
  *
  * Returns: errno
  */
@@ -900,32 +893,36 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
 	BUG_ON(gfs2_glock_is_locked_by_me(ip->i_gl) == NULL);
 
 	ret = gfs2_meta_inode_buffer(ip, &dibh);
-	if (unlikely(ret)) {
-		unlock_page(page);
-		put_page(page);
-		goto failed;
-	}
+	if (unlikely(ret))
+		goto out;
 
-	if (gfs2_is_stuffed(ip))
-		return gfs2_stuffed_write_end(inode, dibh, pos, len, copied, page);
+	if (gfs2_is_stuffed(ip)) {
+		ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+		goto out2;
+	}
 
 	if (!gfs2_is_writeback(ip))
 		gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
 
 	ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+	page = NULL;
 	if (tr->tr_num_buf_new)
 		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
 	else
 		gfs2_trans_add_meta(ip->i_gl, dibh);
 
-
+out2:
 	if (inode == sdp->sd_rindex) {
 		adjust_fs_space(inode);
 		sdp->sd_rindex_uptodate = 0;
 	}
 
 	brelse(dibh);
-failed:
+out:
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
 	gfs2_trans_end(sdp);
 	gfs2_inplace_release(ip);
 	if (ip->i_qadata && ip->i_qadata->qa_qd_num)
-- 
2.14.3

[PATCH 05/10] gfs2: gfs2_stuffed_write_end cleanup (fixup)

[Andreas Gruenbacher]

Revert "Third, make the page argument optional".
---
 fs/gfs2/aops.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index aa2031ff82c8..37410293c1f8 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -815,7 +815,7 @@ static void adjust_fs_space(struct inode *inode)
  * @dibh: The buffer_head containing the on-disk inode
  * @pos: The file position
  * @copied: How much was actually copied by the VFS
- * @page: The (optional) page
+ * @page: The page
  *
  * This copies the data from the page into the inode block after
  * the inode data structure itself.
@@ -830,29 +830,15 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
 	u64 to = pos + copied;
 	void *kaddr;
 	unsigned char *buf = dibh->b_data + sizeof(struct gfs2_dinode);
-	bool page_grabbed = false;
 
 	BUG_ON(pos + copied > gfs2_max_stuffed_size(ip));
 
-	if (!page) {
-		page = find_lock_page(inode->i_mapping, 0);
-		if (WARN_ON(!page))
-			return -ENOMEM;
-		page_grabbed = true;
-	}
-
 	kaddr = kmap_atomic(page);
 	memcpy(buf + pos, kaddr + pos, copied);
 	flush_dcache_page(page);
 	kunmap_atomic(kaddr);
 
 	WARN_ON(!PageUptodate(page));
-
-	if (page_grabbed) {
-		unlock_page(page);
-		put_page(page);
-	}
-
 	if (copied) {
 		if (inode->i_size < to)
 			i_size_write(inode, to);
-- 
2.14.3

[PATCH 06/10] gfs2: Remove ordered write mode handling from gfs2_trans_add_data

[Andreas Gruenbacher]

In journaled data mode, we need to add each buffer head to the current
transaction.  In ordered write mode, we only need to add the inode to
the ordered inode list.  So far, both cases are handled in
gfs2_trans_add_data.  This makes the code look misleading and is
inefficient for small block sizes as well.  Handle both cases separately
instead.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c  |  7 ++++---
 fs/gfs2/bmap.c  | 12 ++++++++----
 fs/gfs2/log.h   |  7 ++++++-
 fs/gfs2/quota.c |  5 ++++-
 fs/gfs2/trans.c | 27 ++++++++-------------------
 5 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 37410293c1f8..42b56c06a97a 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -54,8 +54,7 @@ static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
 			continue;
 		if (start >= to)
 			break;
-		if (gfs2_is_jdata(ip))
-			set_buffer_uptodate(bh);
+		set_buffer_uptodate(bh);
 		gfs2_trans_add_data(ip->i_gl, bh);
 	}
 }
@@ -887,8 +886,10 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
 		goto out2;
 	}
 
-	if (!gfs2_is_writeback(ip))
+	if (gfs2_is_jdata(ip))
 		gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
+	else
+		gfs2_ordered_add_inode(ip);
 
 	ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
 	page = NULL;
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index dded1818bd2d..54b1474d1a8d 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -89,10 +89,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
 		map_bh(bh, inode->i_sb, block);
 
 	set_buffer_uptodate(bh);
-	if (!gfs2_is_jdata(ip))
-		mark_buffer_dirty(bh);
-	if (!gfs2_is_writeback(ip))
+	if (gfs2_is_jdata(ip))
 		gfs2_trans_add_data(ip->i_gl, bh);
+	else {
+		mark_buffer_dirty(bh);
+		gfs2_ordered_add_inode(ip);
+	}
 
 	if (release) {
 		unlock_page(page);
@@ -951,8 +953,10 @@ static int gfs2_block_zero_range(struct inode *inode, loff_t from,
 		err = 0;
 	}
 
-	if (!gfs2_is_writeback(ip))
+	if (gfs2_is_jdata(ip))
 		gfs2_trans_add_data(ip->i_gl, bh);
+	else
+		gfs2_ordered_add_inode(ip);
 
 	zero_user(page, offset, length);
 	mark_buffer_dirty(bh);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index 9499a6049212..32cd0363843b 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/writeback.h>
 #include "incore.h"
+#include "inode.h"
 
 /**
  * gfs2_log_lock - acquire the right to mess with the log manager
@@ -50,8 +51,12 @@ static inline void gfs2_log_pointers_init(struct gfs2_sbd *sdp,
 
 static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
 {
-	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
+	struct gfs2_sbd *sdp;
 
+	if (!gfs2_is_ordered(ip))
+		return;
+
+	sdp = GFS2_SB(&ip->i_inode);
 	if (!test_bit(GIF_ORDERED, &ip->i_flags)) {
 		spin_lock(&sdp->sd_ordered_lock);
 		if (!test_and_set_bit(GIF_ORDERED, &ip->i_flags))
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index e700fb162664..209893588a74 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -735,7 +735,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode *ip, unsigned long index,
 			if (!buffer_uptodate(bh))
 				goto unlock_out;
 		}
-		gfs2_trans_add_data(ip->i_gl, bh);
+		if (gfs2_is_jdata(ip))
+			gfs2_trans_add_data(ip->i_gl, bh);
+		else
+			gfs2_ordered_add_inode(ip);
 
 		/* If we need to write to the next block as well */
 		if (to_write > (bsize - boff)) {
diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index b95ebd166cac..13c038e0c5af 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -142,32 +142,21 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
  * @gl: The inode glock associated with the buffer
  * @bh: The buffer to add
  *
- * This is used in two distinct cases:
- * i) In ordered write mode
- *    We put the data buffer on a list so that we can ensure that it's
- *    synced to disk at the right time
- * ii) In journaled data mode
- *    We need to journal the data block in the same way as metadata in
- *    the functions above. The difference is that here we have a tag
- *    which is two __be64's being the block number (as per meta data)
- *    and a flag which says whether the data block needs escaping or
- *    not. This means we need a new log entry for each 251 or so data
- *    blocks, which isn't an enormous overhead but twice as much as
- *    for normal metadata blocks.
+ * This is used in journaled data mode.
+ * We need to journal the data block in the same way as metadata in
+ * the functions above. The difference is that here we have a tag
+ * which is two __be64's being the block number (as per meta data)
+ * and a flag which says whether the data block needs escaping or
+ * not. This means we need a new log entry for each 251 or so data
+ * blocks, which isn't an enormous overhead but twice as much as
+ * for normal metadata blocks.
  */
 void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
 {
 	struct gfs2_trans *tr = current->journal_info;
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-	struct address_space *mapping = bh->b_page->mapping;
-	struct gfs2_inode *ip = GFS2_I(mapping->host);
 	struct gfs2_bufdata *bd;
 
-	if (!gfs2_is_jdata(ip)) {
-		gfs2_ordered_add_inode(ip);
-		return;
-	}
-
 	lock_buffer(bh);
 	if (buffer_pinned(bh)) {
 		set_bit(TR_TOUCHED, &tr->tr_flags);
-- 
2.14.3

[PATCH 07/10] gfs2: Iomap cleanups and improvements

[Andreas Gruenbacher]

Clean up gfs2_iomap_alloc and gfs2_iomap_get.  Document how
gfs2_iomap_alloc works.  gfs2_iomap_alloc now needs to be called
separately after gfs2_iomap_get where necessary; this will be used later
by iomap write.  Move gfs2_iomap_ops into bmap.c.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c  | 197 ++++++++++++++++++++++++++++++--------------------------
 fs/gfs2/bmap.h  |   4 +-
 fs/gfs2/inode.c |   4 --
 3 files changed, 109 insertions(+), 96 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 54b1474d1a8d..fd9c6c085d0a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -422,22 +422,6 @@ static inline unsigned int gfs2_extent_length(void *start, unsigned int len, __b
 	return (ptr - first);
 }
 
-static inline void bmap_lock(struct gfs2_inode *ip, int create)
-{
-	if (create)
-		down_write(&ip->i_rw_mutex);
-	else
-		down_read(&ip->i_rw_mutex);
-}
-
-static inline void bmap_unlock(struct gfs2_inode *ip, int create)
-{
-	if (create)
-		up_write(&ip->i_rw_mutex);
-	else
-		up_read(&ip->i_rw_mutex);
-}
-
 static inline __be64 *gfs2_indirect_init(struct metapath *mp,
 					 struct gfs2_glock *gl, unsigned int i,
 					 unsigned offset, u64 bn)
@@ -464,15 +448,11 @@ enum alloc_state {
 };
 
 /**
- * gfs2_bmap_alloc - Build a metadata tree of the requested height
+ * gfs2_iomap_alloc - Build a metadata tree of the requested height
  * @inode: The GFS2 inode
- * @lblock: The logical starting block of the extent
- * @bh_map: This is used to return the mapping details
- * @zero_new: True if newly allocated blocks should be zeroed
+ * @iomap: The iomap structure
+ * @flags: iomap flags
  * @mp: The metapath, with proper height information calculated
- * @maxlen: The max number of data blocks to alloc
- * @dblock: Pointer to return the resulting new block
- * @dblks: Pointer to return the number of blocks allocated
  *
  * In this routine we may have to alloc:
  *   i) Indirect blocks to grow the metadata tree height
@@ -485,6 +465,13 @@ enum alloc_state {
  * blocks are available, there will only be one request per bmap call)
  * and uses the state machine to initialise the blocks in order.
  *
+ * Right now, this function will allocate at most one indirect block
+ * worth of data -- with a default block size of 4K, that's slightly
+ * less than 2M.  If this limitation is ever removed to allow huge
+ * allocations, we would probably still want to limit the iomap size we
+ * return to avoid stalling other tasks during huge writes; the next
+ * iomap iteration would then find the blocks already allocated.
+ *
  * Returns: errno on error
  */
 
@@ -511,6 +498,8 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
 
 	gfs2_trans_add_meta(ip->i_gl, dibh);
 
+	down_write(&ip->i_rw_mutex);
+
 	if (mp->mp_fheight == mp->mp_aheight) {
 		struct buffer_head *bh;
 		int eob;
@@ -546,11 +535,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
 	blks = dblks + iblks;
 	i = mp->mp_aheight;
 	do {
-		int error;
 		n = blks - alloced;
-		error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
-		if (error)
-			return error;
+		ret = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
+		if (ret)
+			goto out;
 		alloced += n;
 		if (state != ALLOC_DATA || gfs2_is_jdata(ip))
 			gfs2_trans_add_unrevoke(sdp, bn, n);
@@ -606,7 +594,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
 			dblks = n;
 			ptr = metapointer(end_of_metadata, mp);
 			iomap->addr = bn << inode->i_blkbits;
-			iomap->flags |= IOMAP_F_NEW;
+			iomap->flags |= IOMAP_F_MERGED | IOMAP_F_NEW;
 			while (n-- > 0)
 				*ptr++ = cpu_to_be64(bn++);
 			if (flags & IOMAP_ZERO) {
@@ -625,8 +613,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
 	iomap->length = (u64)dblks << inode->i_blkbits;
 	ip->i_height = mp->mp_fheight;
 	gfs2_add_inode_blocks(&ip->i_inode, alloced);
-	gfs2_dinode_out(ip, mp->mp_bh[0]->b_data);
-	return 0;
+	gfs2_dinode_out(ip, dibh->b_data);
+out:
+	up_write(&ip->i_rw_mutex);
+	return ret;
 }
 
 /**
@@ -635,7 +625,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
  * @lblock: The logical starting block number
  * @mp: The metapath
  *
- * Returns: The hole size in bytes
+ * Returns: The hole size in blocks
  *
  */
 static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
@@ -682,7 +672,7 @@ static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
 		if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
 			(mp->mp_list[hgt - 1])++;
 	}
-	return holesz << inode->i_blkbits;
+	return holesz;
 }
 
 static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
@@ -698,55 +688,46 @@ static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
 }
 
 /**
- * gfs2_iomap_begin - Map blocks from an inode to disk blocks
+ * gfs2_iomap_get - Map blocks from an inode to disk blocks
  * @inode: The inode
  * @pos: Starting position in bytes
  * @length: Length to map, in bytes
  * @flags: iomap flags
  * @iomap: The iomap structure
+ * @mp: The metapath
  *
  * Returns: errno
  */
-int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
-		     unsigned flags, struct iomap *iomap)
+static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
+			  unsigned flags, struct iomap *iomap,
+			  struct metapath *mp)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
-	struct metapath mp = { .mp_aheight = 1, };
 	unsigned int factor = sdp->sd_sb.sb_bsize;
 	const u64 *arr = sdp->sd_heightsize;
 	__be64 *ptr;
 	sector_t lblock;
-	sector_t lend;
+	sector_t lblock_stop;
 	int ret;
 	int eob;
-	unsigned int len;
+	u64 len;
 	struct buffer_head *bh;
 	u8 height;
 
-	trace_gfs2_iomap_start(ip, pos, length, flags);
-	if (!length) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (!length)
+		return -EINVAL;
 
 	if ((flags & IOMAP_REPORT) && gfs2_is_stuffed(ip)) {
-		gfs2_stuffed_iomap(inode, iomap);
-		if (pos >= iomap->length)
+		if (pos >= i_size_read(inode))
 			return -ENOENT;
-		ret = 0;
-		goto out;
+		gfs2_stuffed_iomap(inode, iomap);
+		return 0;
 	}
 
 	lblock = pos >> inode->i_blkbits;
-	lend = (pos + length + sdp->sd_sb.sb_bsize - 1) >> inode->i_blkbits;
-
-	iomap->offset = lblock << inode->i_blkbits;
-	iomap->addr = IOMAP_NULL_ADDR;
-	iomap->type = IOMAP_HOLE;
-	iomap->length = (u64)(lend - lblock) << inode->i_blkbits;
-	iomap->flags = IOMAP_F_MERGED;
-	bmap_lock(ip, 0);
+	lblock_stop = (pos + length - 1) >> inode->i_blkbits;
+	len = lblock_stop - lblock + 1;
 
 	/*
 	 * Directory data blocks have a struct gfs2_meta_header header, so the
@@ -758,61 +739,87 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 		arr = sdp->sd_jheightsize;
 	}
 
-	ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]);
+	down_read(&ip->i_rw_mutex);
+
+	ret = gfs2_meta_inode_buffer(ip, &mp->mp_bh[0]);
 	if (ret)
-		goto out_release;
+		goto unlock;
 
 	height = ip->i_height;
 	while ((lblock + 1) * factor > arr[height])
 		height++;
-	find_metapath(sdp, lblock, &mp, height);
+	find_metapath(sdp, lblock, mp, height);
 	if (height > ip->i_height || gfs2_is_stuffed(ip))
 		goto do_alloc;
 
-	ret = lookup_metapath(ip, &mp);
-	if (ret)
-		goto out_release;
+	ret = lookup_metapath(ip, mp);
+	if (ret < 0)
+		goto unlock;
+	ret = 0;
 
-	if (mp.mp_aheight != ip->i_height)
+	if (mp->mp_aheight != ip->i_height)
 		goto do_alloc;
 
-	ptr = metapointer(ip->i_height - 1, &mp);
+	ptr = metapointer(ip->i_height - 1, mp);
 	if (*ptr == 0)
 		goto do_alloc;
 
-	iomap->type = IOMAP_MAPPED;
 	iomap->addr = be64_to_cpu(*ptr) << inode->i_blkbits;
+	iomap->type = IOMAP_MAPPED;
 
-	bh = mp.mp_bh[ip->i_height - 1];
-	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, lend - lblock, &eob);
+	bh = mp->mp_bh[ip->i_height - 1];
+	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, len, &eob);
+	iomap->flags = IOMAP_F_MERGED;
 	if (eob)
 		iomap->flags |= IOMAP_F_BOUNDARY;
-	iomap->length = (u64)len << inode->i_blkbits;
 
-	ret = 0;
-
-out_release:
-	release_metapath(&mp);
-	bmap_unlock(ip, 0);
 out:
-	trace_gfs2_iomap_end(ip, iomap, ret);
+	iomap->offset = lblock << inode->i_blkbits;
+	iomap->length = len << inode->i_blkbits;
+	iomap->bdev = inode->i_sb->s_bdev;
+unlock:
+	up_read(&ip->i_rw_mutex);
 	return ret;
 
 do_alloc:
 	if (!(flags & IOMAP_WRITE)) {
 		if (pos >= i_size_read(inode)) {
 			ret = -ENOENT;
-			goto out_release;
+			goto unlock;
 		}
-		ret = 0;
-		iomap->length = hole_size(inode, lblock, &mp);
-		goto out_release;
+		len = hole_size(inode, lblock, mp);
 	}
+	iomap->addr = IOMAP_NULL_ADDR;
+	iomap->type = IOMAP_HOLE;
+	iomap->flags = 0;
+	goto out;
+}
 
-	ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
-	goto out_release;
+static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
+			    unsigned flags, struct iomap *iomap)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct metapath mp = { .mp_aheight = 1, };
+	int ret;
+
+	trace_gfs2_iomap_start(ip, pos, length, flags);
+	if (flags & IOMAP_WRITE) {
+		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+		if (!ret && iomap->type == IOMAP_HOLE)
+			ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+		release_metapath(&mp);
+	} else {
+		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+		release_metapath(&mp);
+	}
+	trace_gfs2_iomap_end(ip, iomap, ret);
+	return ret;
 }
 
+const struct iomap_ops gfs2_iomap_ops = {
+	.iomap_begin = gfs2_iomap_begin,
+};
+
 /**
  * gfs2_block_map - Map a block from an inode to a disk block
  * @inode: The inode
@@ -831,27 +838,37 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 		   struct buffer_head *bh_map, int create)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
+	loff_t pos = (loff_t)lblock << inode->i_blkbits;
+	loff_t length = bh_map->b_size;
+	struct metapath mp = { .mp_aheight = 1, };
 	struct iomap iomap;
-	int ret, flags = 0;
+	int ret;
 
 	clear_buffer_mapped(bh_map);
 	clear_buffer_new(bh_map);
 	clear_buffer_boundary(bh_map);
 	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
 
-	if (create)
-		flags |= IOMAP_WRITE;
-	if (buffer_zeronew(bh_map))
-		flags |= IOMAP_ZERO;
-	ret = gfs2_iomap_begin(inode, (loff_t)lblock << inode->i_blkbits,
-			       bh_map->b_size, flags, &iomap);
-	if (ret) {
-		if (!create && ret == -ENOENT) {
-			/* Return unmapped buffer beyond the end of file.  */
+	if (create) {
+		int flags = IOMAP_WRITE;
+		if (buffer_zeronew(bh_map))
+			flags |= IOMAP_ZERO;
+		ret = gfs2_iomap_get(inode, pos, length, flags, &iomap, &mp);
+		if (!ret && iomap.type == IOMAP_HOLE)
+			ret = gfs2_iomap_alloc(inode, &iomap, flags, &mp);
+		release_metapath(&mp);
+	} else {
+		ret = gfs2_iomap_get(inode, pos, length, 0, &iomap, &mp);
+		release_metapath(&mp);
+
+		/* Return unmapped buffer beyond the end of file. */
+		if (ret == -ENOENT) {
 			ret = 0;
+			goto out;
 		}
-		goto out;
 	}
+	if (ret)
+		goto out;
 
 	if (iomap.length > bh_map->b_size) {
 		iomap.length = bh_map->b_size;
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index c3402fe00653..5d563c29cb0a 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -46,11 +46,11 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip,
 	}
 }
 
+extern const struct iomap_ops gfs2_iomap_ops;
+
 extern int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page);
 extern int gfs2_block_map(struct inode *inode, sector_t lblock,
 			  struct buffer_head *bh, int create);
-extern int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
-			    unsigned flags, struct iomap *iomap);
 extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
 			   u64 *dblock, unsigned *extlen);
 extern int gfs2_setattr_size(struct inode *inode, u64 size);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 20281992d456..5cc193e5737b 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2015,10 +2015,6 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 	return 0;
 }
 
-const struct iomap_ops gfs2_iomap_ops = {
-	.iomap_begin = gfs2_iomap_begin,
-};
-
 static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		       u64 start, u64 len)
 {
-- 
2.14.3

[PATCH 08/10] iomap: New iomap_written operation

[Andreas Gruenbacher]

Add a callback to iomap_file_buffered_write that's called whenever
writing to a page has completed.  This is needed for implementing data
journaling: in the data journaling case, pages are written into the
journal before being written back to their proper on-disk locations.

(So far, the only user of iomap_file_buffered_write is xfs, which
doesn't do data journaling.)

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/iomap.c            | 21 +++++++++++++++++++--
 include/linux/iomap.h |  9 +++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index 47d29ccffaef..98903be66c35 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -149,11 +149,21 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
 	return ret;
 }
 
+struct iomap_write_data {
+	struct iov_iter *iter;
+	const struct iomap_ops *ops;
+};
+
 static loff_t
 iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 		struct iomap *iomap)
 {
-	struct iov_iter *i = data;
+	struct iomap_write_data *d = data;
+	struct iov_iter *i = d->iter;
+	void (*iomap_written)(struct inode *inode, struct page *page,
+			      unsigned int offset, unsigned int length,
+			      unsigned int written) =
+		d->ops->iomap_written;
 	long status = 0;
 	ssize_t written = 0;
 	unsigned int flags = AOP_FLAG_NOFS;
@@ -198,6 +208,9 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 
 		flush_dcache_page(page);
 
+		if (iomap_written)
+			iomap_written(inode, page, offset, bytes, copied);
+
 		status = iomap_write_end(inode, pos, bytes, copied, page);
 		if (unlikely(status < 0))
 			break;
@@ -235,10 +248,14 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *iter,
 {
 	struct inode *inode = iocb->ki_filp->f_mapping->host;
 	loff_t pos = iocb->ki_pos, ret = 0, written = 0;
+	struct iomap_write_data data = {
+		.iter = iter,
+		.ops = ops,
+	};
 
 	while (iov_iter_count(iter)) {
 		ret = iomap_apply(inode, pos, iov_iter_count(iter),
-				IOMAP_WRITE, ops, iter, iomap_write_actor);
+				IOMAP_WRITE, ops, &data, iomap_write_actor);
 		if (ret <= 0)
 			break;
 		pos += ret;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 19a07de28212..042b8c8df44b 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -61,6 +61,8 @@ struct iomap {
 #define IOMAP_DIRECT		(1 << 4) /* direct I/O */
 #define IOMAP_NOWAIT		(1 << 5) /* Don't wait for writeback */
 
+struct page;
+
 struct iomap_ops {
 	/*
 	 * Return the existing mapping at pos, or reserve space starting at
@@ -70,6 +72,13 @@ struct iomap_ops {
 	int (*iomap_begin)(struct inode *inode, loff_t pos, loff_t length,
 			unsigned flags, struct iomap *iomap);
 
+	/*
+	 * Called after writing to a page has completed.
+	 */
+	void (*iomap_written)(struct inode *inode, struct page *page,
+			      unsigned int offset, unsigned int length,
+			      unsigned int written);
+
 	/*
 	 * Commit and/or unreserve space previous allocated using iomap_begin.
 	 * Written indicates the length of the successful write operation which
-- 
2.14.3

Re: [PATCH 08/10] iomap: New iomap_written operation

[Dave Chinner]

On Thu, Jan 11, 2018 at 10:15:02PM +0100, Andreas Gruenbacher wrote:
> Add a callback to iomap_file_buffered_write that's called whenever
> writing to a page has completed.  This is needed for implementing data
> journaling: in the data journaling case, pages are written into the
> journal before being written back to their proper on-disk locations.
> 
> (So far, the only user of iomap_file_buffered_write is xfs, which
> doesn't do data journaling.)
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/iomap.c            | 21 +++++++++++++++++++--
>  include/linux/iomap.h |  9 +++++++++
>  2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 47d29ccffaef..98903be66c35 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -149,11 +149,21 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
>  	return ret;
>  }
>  
> +struct iomap_write_data {
> +	struct iov_iter *iter;
> +	const struct iomap_ops *ops;
> +};
> +
>  static loff_t
>  iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  		struct iomap *iomap)
>  {
> -	struct iov_iter *i = data;
> +	struct iomap_write_data *d = data;
> +	struct iov_iter *i = d->iter;
> +	void (*iomap_written)(struct inode *inode, struct page *page,
> +			      unsigned int offset, unsigned int length,
> +			      unsigned int written) =
> +		d->ops->iomap_written;
>  	long status = 0;
>  	ssize_t written = 0;
>  	unsigned int flags = AOP_FLAG_NOFS;
> @@ -198,6 +208,9 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  
>  		flush_dcache_page(page);
>  
> +		if (iomap_written)
> +			iomap_written(inode, page, offset, bytes, copied);
> +
>  		status = iomap_write_end(inode, pos, bytes, copied, page);
>  		if (unlikely(status < 0))
>  			break;

This looks like it should replace iomap_write_end() as it has
pretty much the same parameters are passed to it.

i.e. it seems to me like a better solution would be to add
->write_begin/->write_end ops to the struct iomap_ops and have
existing implementations set them up as iomap_write_begin()/
iomap_write_end(). Then gfs2 can do it's special little extra bit
and then call iomap_write_end() in the one call...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 09/10] gfs2: Implement buffered iomap write support (1)

[Andreas Gruenbacher]

With the traditional page-based writes, blocks are allocated separately
for each page written to.  With iomap writes, we can allocate a lot more
blocks at once, with a fraction of the allocation overhead for each
page.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c |   7 +-
 fs/gfs2/aops.h |  18 ++++++
 fs/gfs2/bmap.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/gfs2/file.c |  47 +++++++++++++-
 4 files changed, 259 insertions(+), 10 deletions(-)
 create mode 100644 fs/gfs2/aops.h

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 42b56c06a97a..a4ce16b362bb 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -36,10 +36,11 @@
 #include "super.h"
 #include "util.h"
 #include "glops.h"
+#include "aops.h"
 
 
-static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
-				   unsigned int from, unsigned int len)
+void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+			    unsigned int from, unsigned int len)
 {
 	struct buffer_head *head = page_buffers(page);
 	unsigned int bsize = head->b_size;
@@ -773,7 +774,7 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
  * adjust_fs_space - Adjusts the free space available due to gfs2_grow
  * @inode: the rindex inode
  */
-static void adjust_fs_space(struct inode *inode)
+void adjust_fs_space(struct inode *inode)
 {
 	struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
new file mode 100644
index 000000000000..9a2fa61d8ca4
--- /dev/null
+++ b/fs/gfs2/aops.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc.  All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License version 2.
+ */
+
+#ifndef __AOPS_DOT_H__
+#define __AOPS_DOT_H__
+
+#include "incore.h"
+
+extern void adjust_fs_space(struct inode *inode);
+extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+				   unsigned int from, unsigned int len);
+
+#endif /* __AOPS_DOT_H__ */
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 0d6d81227477..382d5fb135c1 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -28,6 +28,7 @@
 #include "trans.h"
 #include "dir.h"
 #include "util.h"
+#include "aops.h"
 #include "trace_gfs2.h"
 
 /* This doesn't need to be that large as max 64 bit pointers in a 4k
@@ -41,6 +42,8 @@ struct metapath {
 	int mp_aheight; /* actual height (lookup height) */
 };
 
+static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length);
+
 /**
  * gfs2_unstuffer_page - unstuff a stuffed inode into a block cached by a page
  * @ip: the inode
@@ -375,7 +378,7 @@ static int fillup_metapath(struct gfs2_inode *ip, struct metapath *mp, int h)
 	return mp->mp_aheight - x - 1;
 }
 
-static inline void release_metapath(struct metapath *mp)
+static void release_metapath(struct metapath *mp)
 {
 	int i;
 
@@ -383,6 +386,7 @@ static inline void release_metapath(struct metapath *mp)
 		if (mp->mp_bh[i] == NULL)
 			break;
 		brelse(mp->mp_bh[i]);
+		mp->mp_bh[i] = NULL;
 	}
 }
 
@@ -796,6 +800,172 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 	goto out;
 }
 
+static int gfs2_write_lock(struct inode *inode)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	int error;
+
+	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &ip->i_gh);
+	error = gfs2_glock_nq(&ip->i_gh);
+	if (error)
+		goto out_uninit;
+	if (&ip->i_inode == sdp->sd_rindex) {
+		struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+		error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE,
+					   GL_NOCACHE, &m_ip->i_gh);
+		if (error)
+			goto out_unlock;
+	}
+	return 0;
+
+out_unlock:
+	gfs2_glock_dq(&ip->i_gh);
+out_uninit:
+	gfs2_holder_uninit(&ip->i_gh);
+	return error;
+}
+
+static void gfs2_write_unlock(struct inode *inode)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+
+	if (&ip->i_inode == sdp->sd_rindex) {
+		struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+		gfs2_glock_dq_uninit(&m_ip->i_gh);
+	}
+	gfs2_glock_dq_uninit(&ip->i_gh);
+}
+
+static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length,
+				  unsigned flags, struct iomap *iomap)
+{
+	struct metapath mp = { .mp_aheight = 1, };
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
+	int alloc_required;
+	int ret;
+
+	ret = gfs2_write_lock(inode);
+	if (ret)
+		return ret;
+
+	if (gfs2_is_stuffed(ip)) {
+		if (pos + length <= gfs2_max_stuffed_size(ip)) {
+			ret = -ENOTBLK;
+			goto out_unlock;
+		}
+	}
+
+	ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+	if (ret)
+		goto out_release;
+	alloc_required = iomap->type != IOMAP_MAPPED;
+
+	if (alloc_required || gfs2_is_jdata(ip))
+		gfs2_write_calc_reserv(ip, iomap->length, &data_blocks, &ind_blocks);
+
+	if (alloc_required) {
+		struct gfs2_alloc_parms ap = { .target = data_blocks + ind_blocks };
+		ret = gfs2_quota_lock_check(ip, &ap);
+		if (ret)
+			goto out_release;
+
+		ret = gfs2_inplace_reserve(ip, &ap);
+		if (ret)
+			goto out_qunlock;
+	}
+
+	rblocks = RES_DINODE + ind_blocks;
+	if (gfs2_is_jdata(ip))
+		rblocks += data_blocks;
+	if (ind_blocks || data_blocks)
+		rblocks += RES_STATFS + RES_QUOTA;
+	if (inode == sdp->sd_rindex)
+		rblocks += 2 * RES_STATFS;
+	if (alloc_required)
+		rblocks += gfs2_rg_blocks(ip, data_blocks + ind_blocks);
+
+	ret = gfs2_trans_begin(sdp, rblocks, iomap->length >> inode->i_blkbits);
+	if (ret)
+		goto out_trans_fail;
+
+	if (gfs2_is_stuffed(ip)) {
+		ret = gfs2_unstuff_dinode(ip, NULL);
+		if (ret)
+			goto out_trans_end;
+		release_metapath(&mp);
+		ret = gfs2_iomap_get(inode, iomap->offset, iomap->length,
+				     flags, iomap, &mp);
+		if (ret)
+			goto out_trans_end;
+	}
+
+	if (iomap->type != IOMAP_MAPPED) {
+		ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+		if (ret) {
+			gfs2_trans_end(sdp);
+			punch_hole(ip, iomap->offset, iomap->length);
+			goto out_trans_fail;
+		}
+	}
+	release_metapath(&mp);
+	return 0;
+
+out_trans_end:
+	gfs2_trans_end(sdp);
+out_trans_fail:
+	if (alloc_required) {
+		gfs2_inplace_release(ip);
+out_qunlock:
+		gfs2_quota_unlock(ip);
+	}
+out_release:
+	release_metapath(&mp);
+out_unlock:
+	gfs2_write_unlock(inode);
+	return ret;
+}
+
+static int gfs2_iomap_write_end(struct inode *inode, loff_t pos, loff_t length,
+				ssize_t written, struct iomap *iomap)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	struct gfs2_trans *tr = current->journal_info;
+
+	BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
+
+	gfs2_ordered_add_inode(ip);
+
+	if (tr->tr_num_buf_new)
+		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+
+	if (inode == sdp->sd_rindex) {
+		adjust_fs_space(inode);
+		sdp->sd_rindex_uptodate = 0;
+	}
+
+	if (length != written && (iomap->flags & IOMAP_F_NEW)) {
+		/* Deallocate blocks that were just allocated. */
+		pos += written;
+		length -= written;
+		truncate_pagecache_range(inode, pos, length);
+		punch_hole(ip, pos, length);
+	}
+
+	gfs2_trans_end(sdp);
+	gfs2_inplace_release(ip);
+	if (ip->i_qadata && ip->i_qadata->qa_qd_num)
+		gfs2_quota_unlock(ip);
+	gfs2_write_unlock(inode);
+	return 0;
+}
+
 static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 			    unsigned flags, struct iomap *iomap)
 {
@@ -805,10 +975,7 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 
 	trace_gfs2_iomap_start(ip, pos, length, flags);
 	if (flags & IOMAP_WRITE) {
-		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
-		if (!ret && iomap->type == IOMAP_HOLE)
-			ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
-		release_metapath(&mp);
+		ret = gfs2_iomap_write_begin(inode, pos, length, flags, iomap);
 	} else {
 		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
 		release_metapath(&mp);
@@ -817,8 +984,28 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 	return ret;
 }
 
+static void gfs2_iomap_written(struct inode *inode, struct page *page,
+			       unsigned int offset, unsigned int length,
+			       unsigned int written)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+
+	if (gfs2_is_jdata(ip))
+		gfs2_page_add_databufs(ip, page, offset, length);
+}
+
+static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length,
+			  ssize_t written, unsigned flags, struct iomap *iomap)
+{
+	if (flags & IOMAP_WRITE)
+		return gfs2_iomap_write_end(inode, pos, length, written, iomap);
+	return 0;
+}
+
 const struct iomap_ops gfs2_iomap_ops = {
 	.iomap_begin = gfs2_iomap_begin,
+	.iomap_written = gfs2_iomap_written,
+	.iomap_end = gfs2_iomap_end,
 };
 
 /**
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index bd60dc682676..94d44e30d188 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -26,6 +26,7 @@
 #include <linux/dlm.h>
 #include <linux/dlm_plock.h>
 #include <linux/delay.h>
+#include <linux/backing-dev.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -686,6 +687,43 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
 	return ret ? ret : ret1;
 }
 
+static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file->f_mapping->host;
+	ssize_t ret;
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, from);
+	if (ret <= 0)
+		goto out;
+
+	ret = file_remove_privs(file);
+	if (ret)
+		goto out;
+
+	ret = file_update_time(file);
+	if (ret)
+		goto out;
+
+	/* We can write back this queue in page reclaim */
+	current->backing_dev_info = inode_to_bdi(inode);
+
+	ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+
+	current->backing_dev_info = NULL;
+
+out:
+	inode_unlock(inode);
+	if (likely(ret > 0)) {
+		iocb->ki_pos += ret;
+
+		/* Handle various SYNC-type writes */
+		ret = generic_write_sync(iocb, ret);
+	}
+	return ret;
+}
+
 /**
  * gfs2_file_write_iter - Perform a write to a file
  * @iocb: The io context
@@ -704,7 +742,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
 	struct gfs2_inode *ip = GFS2_I(file_inode(file));
-	int ret;
+	ssize_t ret;
 
 	ret = gfs2_rsqa_alloc(ip);
 	if (ret)
@@ -721,7 +759,12 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		gfs2_glock_dq_uninit(&gh);
 	}
 
-	return generic_file_write_iter(iocb, from);
+	if (iocb->ki_flags & IOCB_DIRECT)
+		return generic_file_write_iter(iocb, from);
+	ret = gfs2_file_buffered_write(iocb, from);
+	if (ret == -ENOTBLK)
+		ret = generic_file_write_iter(iocb, from);
+	return ret;
 }
 
 static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
-- 
2.14.3

Re: [PATCH 09/10] gfs2: Implement buffered iomap write support (1)

[Andreas Grünbacher]

2018-01-11 22:15 GMT+01:00 Andreas Gruenbacher <agruenba@redhat.com>:
> With the traditional page-based writes, blocks are allocated separately
> for each page written to.  With iomap writes, we can allocate a lot more
> blocks at once, with a fraction of the allocation overhead for each
> page.

Oops, please ignore this patch.

Thanks,
Andreas

[PATCH 09/10] gfs2: Implement iomap buffered write support (1)

[Andreas Gruenbacher]

With the traditional page-based writes, blocks are allocated separately
for each page written to.  With iomap writes, we can allocate a lot more
blocks at once, with a fraction of the allocation overhead for each
page.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c |   7 +-
 fs/gfs2/aops.h |  18 ++++++
 fs/gfs2/bmap.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/gfs2/file.c |  47 +++++++++++++-
 4 files changed, 259 insertions(+), 10 deletions(-)
 create mode 100644 fs/gfs2/aops.h

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 42b56c06a97a..a4ce16b362bb 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -36,10 +36,11 @@
 #include "super.h"
 #include "util.h"
 #include "glops.h"
+#include "aops.h"
 
 
-static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
-				   unsigned int from, unsigned int len)
+void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+			    unsigned int from, unsigned int len)
 {
 	struct buffer_head *head = page_buffers(page);
 	unsigned int bsize = head->b_size;
@@ -773,7 +774,7 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
  * adjust_fs_space - Adjusts the free space available due to gfs2_grow
  * @inode: the rindex inode
  */
-static void adjust_fs_space(struct inode *inode)
+void adjust_fs_space(struct inode *inode)
 {
 	struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
new file mode 100644
index 000000000000..9a2fa61d8ca4
--- /dev/null
+++ b/fs/gfs2/aops.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc.  All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License version 2.
+ */
+
+#ifndef __AOPS_DOT_H__
+#define __AOPS_DOT_H__
+
+#include "incore.h"
+
+extern void adjust_fs_space(struct inode *inode);
+extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+				   unsigned int from, unsigned int len);
+
+#endif /* __AOPS_DOT_H__ */
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index fd9c6c085d0a..e070cb9a8094 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -28,6 +28,7 @@
 #include "trans.h"
 #include "dir.h"
 #include "util.h"
+#include "aops.h"
 #include "trace_gfs2.h"
 
 /* This doesn't need to be that large as max 64 bit pointers in a 4k
@@ -41,6 +42,8 @@ struct metapath {
 	int mp_aheight; /* actual height (lookup height) */
 };
 
+static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length);
+
 /**
  * gfs2_unstuffer_page - unstuff a stuffed inode into a block cached by a page
  * @ip: the inode
@@ -375,7 +378,7 @@ static int fillup_metapath(struct gfs2_inode *ip, struct metapath *mp, int h)
 	return mp->mp_aheight - x - 1;
 }
 
-static inline void release_metapath(struct metapath *mp)
+static void release_metapath(struct metapath *mp)
 {
 	int i;
 
@@ -383,6 +386,7 @@ static inline void release_metapath(struct metapath *mp)
 		if (mp->mp_bh[i] == NULL)
 			break;
 		brelse(mp->mp_bh[i]);
+		mp->mp_bh[i] = NULL;
 	}
 }
 
@@ -795,6 +799,172 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 	goto out;
 }
 
+static int gfs2_write_lock(struct inode *inode)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	int error;
+
+	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &ip->i_gh);
+	error = gfs2_glock_nq(&ip->i_gh);
+	if (error)
+		goto out_uninit;
+	if (&ip->i_inode == sdp->sd_rindex) {
+		struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+		error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE,
+					   GL_NOCACHE, &m_ip->i_gh);
+		if (error)
+			goto out_unlock;
+	}
+	return 0;
+
+out_unlock:
+	gfs2_glock_dq(&ip->i_gh);
+out_uninit:
+	gfs2_holder_uninit(&ip->i_gh);
+	return error;
+}
+
+static void gfs2_write_unlock(struct inode *inode)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+
+	if (&ip->i_inode == sdp->sd_rindex) {
+		struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+		gfs2_glock_dq_uninit(&m_ip->i_gh);
+	}
+	gfs2_glock_dq_uninit(&ip->i_gh);
+}
+
+static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length,
+				  unsigned flags, struct iomap *iomap)
+{
+	struct metapath mp = { .mp_aheight = 1, };
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
+	int alloc_required;
+	int ret;
+
+	ret = gfs2_write_lock(inode);
+	if (ret)
+		return ret;
+
+	if (gfs2_is_stuffed(ip)) {
+		if (pos + length <= gfs2_max_stuffed_size(ip)) {
+			ret = -ENOTBLK;
+			goto out_unlock;
+		}
+	}
+
+	ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+	if (ret)
+		goto out_release;
+	alloc_required = iomap->type != IOMAP_MAPPED;
+
+	if (alloc_required || gfs2_is_jdata(ip))
+		gfs2_write_calc_reserv(ip, iomap->length, &data_blocks, &ind_blocks);
+
+	if (alloc_required) {
+		struct gfs2_alloc_parms ap = { .target = data_blocks + ind_blocks };
+		ret = gfs2_quota_lock_check(ip, &ap);
+		if (ret)
+			goto out_release;
+
+		ret = gfs2_inplace_reserve(ip, &ap);
+		if (ret)
+			goto out_qunlock;
+	}
+
+	rblocks = RES_DINODE + ind_blocks;
+	if (gfs2_is_jdata(ip))
+		rblocks += data_blocks;
+	if (ind_blocks || data_blocks)
+		rblocks += RES_STATFS + RES_QUOTA;
+	if (inode == sdp->sd_rindex)
+		rblocks += 2 * RES_STATFS;
+	if (alloc_required)
+		rblocks += gfs2_rg_blocks(ip, data_blocks + ind_blocks);
+
+	ret = gfs2_trans_begin(sdp, rblocks, iomap->length >> inode->i_blkbits);
+	if (ret)
+		goto out_trans_fail;
+
+	if (gfs2_is_stuffed(ip)) {
+		ret = gfs2_unstuff_dinode(ip, NULL);
+		if (ret)
+			goto out_trans_end;
+		release_metapath(&mp);
+		ret = gfs2_iomap_get(inode, iomap->offset, iomap->length,
+				     flags, iomap, &mp);
+		if (ret)
+			goto out_trans_end;
+	}
+
+	if (iomap->type != IOMAP_MAPPED) {
+		ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+		if (ret) {
+			gfs2_trans_end(sdp);
+			punch_hole(ip, iomap->offset, iomap->length);
+			goto out_trans_fail;
+		}
+	}
+	release_metapath(&mp);
+	return 0;
+
+out_trans_end:
+	gfs2_trans_end(sdp);
+out_trans_fail:
+	if (alloc_required) {
+		gfs2_inplace_release(ip);
+out_qunlock:
+		gfs2_quota_unlock(ip);
+	}
+out_release:
+	release_metapath(&mp);
+out_unlock:
+	gfs2_write_unlock(inode);
+	return ret;
+}
+
+static int gfs2_iomap_write_end(struct inode *inode, loff_t pos, loff_t length,
+				ssize_t written, struct iomap *iomap)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	struct gfs2_trans *tr = current->journal_info;
+
+	BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
+
+	gfs2_ordered_add_inode(ip);
+
+	if (tr->tr_num_buf_new)
+		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+
+	if (inode == sdp->sd_rindex) {
+		adjust_fs_space(inode);
+		sdp->sd_rindex_uptodate = 0;
+	}
+
+	if (length != written && (iomap->flags & IOMAP_F_NEW)) {
+		/* Deallocate blocks that were just allocated. */
+		pos += written;
+		length -= written;
+		truncate_pagecache_range(inode, pos, pos + length - 1);
+		punch_hole(ip, pos, length);
+	}
+
+	gfs2_trans_end(sdp);
+	gfs2_inplace_release(ip);
+	if (ip->i_qadata && ip->i_qadata->qa_qd_num)
+		gfs2_quota_unlock(ip);
+	gfs2_write_unlock(inode);
+	return 0;
+}
+
 static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 			    unsigned flags, struct iomap *iomap)
 {
@@ -804,10 +974,7 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 
 	trace_gfs2_iomap_start(ip, pos, length, flags);
 	if (flags & IOMAP_WRITE) {
-		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
-		if (!ret && iomap->type == IOMAP_HOLE)
-			ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
-		release_metapath(&mp);
+		ret = gfs2_iomap_write_begin(inode, pos, length, flags, iomap);
 	} else {
 		ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
 		release_metapath(&mp);
@@ -816,8 +983,28 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 	return ret;
 }
 
+static void gfs2_iomap_written(struct inode *inode, struct page *page,
+			       unsigned int offset, unsigned int length,
+			       unsigned int written)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+
+	if (gfs2_is_jdata(ip))
+		gfs2_page_add_databufs(ip, page, offset, length);
+}
+
+static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length,
+			  ssize_t written, unsigned flags, struct iomap *iomap)
+{
+	if (flags & IOMAP_WRITE)
+		return gfs2_iomap_write_end(inode, pos, length, written, iomap);
+	return 0;
+}
+
 const struct iomap_ops gfs2_iomap_ops = {
 	.iomap_begin = gfs2_iomap_begin,
+	.iomap_written = gfs2_iomap_written,
+	.iomap_end = gfs2_iomap_end,
 };
 
 /**
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index bd60dc682676..94d44e30d188 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -26,6 +26,7 @@
 #include <linux/dlm.h>
 #include <linux/dlm_plock.h>
 #include <linux/delay.h>
+#include <linux/backing-dev.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -686,6 +687,43 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
 	return ret ? ret : ret1;
 }
 
+static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file->f_mapping->host;
+	ssize_t ret;
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, from);
+	if (ret <= 0)
+		goto out;
+
+	ret = file_remove_privs(file);
+	if (ret)
+		goto out;
+
+	ret = file_update_time(file);
+	if (ret)
+		goto out;
+
+	/* We can write back this queue in page reclaim */
+	current->backing_dev_info = inode_to_bdi(inode);
+
+	ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+
+	current->backing_dev_info = NULL;
+
+out:
+	inode_unlock(inode);
+	if (likely(ret > 0)) {
+		iocb->ki_pos += ret;
+
+		/* Handle various SYNC-type writes */
+		ret = generic_write_sync(iocb, ret);
+	}
+	return ret;
+}
+
 /**
  * gfs2_file_write_iter - Perform a write to a file
  * @iocb: The io context
@@ -704,7 +742,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
 	struct gfs2_inode *ip = GFS2_I(file_inode(file));
-	int ret;
+	ssize_t ret;
 
 	ret = gfs2_rsqa_alloc(ip);
 	if (ret)
@@ -721,7 +759,12 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		gfs2_glock_dq_uninit(&gh);
 	}
 
-	return generic_file_write_iter(iocb, from);
+	if (iocb->ki_flags & IOCB_DIRECT)
+		return generic_file_write_iter(iocb, from);
+	ret = gfs2_file_buffered_write(iocb, from);
+	if (ret == -ENOTBLK)
+		ret = generic_file_write_iter(iocb, from);
+	return ret;
 }
 
 static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
-- 
2.14.3

[PATCH 10/10] gfs2: Implement buffered iomap write support (2)

[Andreas Gruenbacher]

Instead of falling back to generic_file_write_iter when writing to a
stuffed file that stays stuffed, implement that case separately.  We
eventually want to get rid of the remaining users of gfs2_write_begin +
gfs2_write_end so that those functions can eventually be removed, and
generic_file_write_iter uses that interface.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/gfs2/aops.h |  1 +
 fs/gfs2/bmap.c | 13 ++++----
 fs/gfs2/bmap.h |  1 +
 fs/gfs2/file.c | 10 ++++---
 5 files changed, 108 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index a4ce16b362bb..852c9b5ecef2 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -22,6 +22,7 @@
 #include <linux/backing-dev.h>
 #include <linux/uio.h>
 #include <trace/events/writeback.h>
+#include <linux/sched/signal.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -847,6 +848,99 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
 	return copied;
 }
 
+ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	loff_t pos = iocb->ki_pos;
+	struct page *page = NULL;
+	ssize_t written = 0, ret;
+
+	BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
+	BUG_ON(!gfs2_is_stuffed(ip));
+
+	ret = gfs2_trans_begin(sdp, RES_DINODE, 0);
+	if (ret)
+		return ret;
+
+	do {
+		struct buffer_head *dibh;
+		unsigned long offset;
+		unsigned long bytes;
+		size_t copied;
+
+		offset = pos & (PAGE_SIZE - 1);
+		bytes = min_t(unsigned long, PAGE_SIZE - offset,
+					     iov_iter_count(from));
+again:
+		/*
+		 * Bring in the user page that we will copy from _first_.
+		 * Otherwise there's a nasty deadlock on copying from the
+		 * same page as we're writing to, without it being marked
+		 * up-to-date.
+		 *
+		 * Not only is this an optimisation, but it is also required
+		 * to check that the address is actually valid, when atomic
+		 * usercopies are used, below.
+		 */
+		if (unlikely(iov_iter_fault_in_readable(from, bytes))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		if (fatal_signal_pending(current)) {
+			ret = -EINTR;
+			goto out;
+		}
+
+		page = grab_cache_page_write_begin(inode->i_mapping, pos >> PAGE_SHIFT, AOP_FLAG_NOFS);
+		if (!page)
+			return -ENOMEM;
+
+		if (!PageUptodate(page)) {
+			ret = stuffed_readpage(ip, page);
+			if (ret)
+				goto out;
+		}
+
+		if (mapping_writably_mapped(inode->i_mapping))
+			flush_dcache_page(page);
+
+		copied = iov_iter_copy_from_user_atomic(page, from, pos, bytes);
+
+		flush_dcache_page(page);
+
+		ret = gfs2_meta_inode_buffer(ip, &dibh);
+		if (ret)
+			goto out;
+		ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+		brelse(dibh);
+		if (ret)
+			goto out;
+
+		unlock_page(page);
+		put_page(page);
+		page = NULL;
+
+		iov_iter_advance(from, copied);
+		if (unlikely(copied == 0)) {
+			bytes = iov_iter_single_seg_count(from);
+			goto again;
+		}
+		pos += copied;
+		written += copied;
+	} while (iov_iter_count(from));
+
+out:
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
+	gfs2_trans_end(sdp);
+	return written ? written : ret;
+}
+
 /**
  * gfs2_write_end
  * @file: The file to write to
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
index 9a2fa61d8ca4..600dbe14c4b7 100644
--- a/fs/gfs2/aops.h
+++ b/fs/gfs2/aops.h
@@ -11,6 +11,7 @@
 
 #include "incore.h"
 
+extern ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from);
 extern void adjust_fs_space(struct inode *inode);
 extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
 				   unsigned int from, unsigned int len);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 382d5fb135c1..882a6b87d971 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -827,7 +827,7 @@ static int gfs2_write_lock(struct inode *inode)
 	return error;
 }
 
-static void gfs2_write_unlock(struct inode *inode)
+void gfs2_write_unlock(struct inode *inode)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -856,14 +856,14 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 
 	if (gfs2_is_stuffed(ip)) {
 		if (pos + length <= gfs2_max_stuffed_size(ip)) {
-			ret = -ENOTBLK;
-			goto out_unlock;
+			/* Keep the inode locked! */
+			return -ENOTBLK;
 		}
 	}
 
 	ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
 	if (ret)
-		goto out_release;
+		goto out_unlock;
 	alloc_required = iomap->type != IOMAP_MAPPED;
 
 	if (alloc_required || gfs2_is_jdata(ip))
@@ -873,7 +873,7 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 		struct gfs2_alloc_parms ap = { .target = data_blocks + ind_blocks };
 		ret = gfs2_quota_lock_check(ip, &ap);
 		if (ret)
-			goto out_release;
+			goto out_unlock;
 
 		ret = gfs2_inplace_reserve(ip, &ap);
 		if (ret)
@@ -924,9 +924,8 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 out_qunlock:
 		gfs2_quota_unlock(ip);
 	}
-out_release:
-	release_metapath(&mp);
 out_unlock:
+	release_metapath(&mp);
 	gfs2_write_unlock(inode);
 	return ret;
 }
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index 5d563c29cb0a..7aa4cf56a6b2 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -62,5 +62,6 @@ extern int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset,
 extern int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd);
 extern void gfs2_free_journal_extents(struct gfs2_jdesc *jd);
 extern int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length);
+extern void gfs2_write_unlock(struct inode *inode);
 
 #endif /* __BMAP_DOT_H__ */
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 94d44e30d188..020c6d8abaf3 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -31,6 +31,7 @@
 #include "gfs2.h"
 #include "incore.h"
 #include "bmap.h"
+#include "aops.h"
 #include "dir.h"
 #include "glock.h"
 #include "glops.h"
@@ -710,6 +711,10 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *fro
 	current->backing_dev_info = inode_to_bdi(inode);
 
 	ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+	if (ret == -ENOTBLK) {
+		ret = gfs2_stuffed_write(iocb, from);
+		gfs2_write_unlock(inode);
+	}
 
 	current->backing_dev_info = NULL;
 
@@ -761,10 +766,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	if (iocb->ki_flags & IOCB_DIRECT)
 		return generic_file_write_iter(iocb, from);
-	ret = gfs2_file_buffered_write(iocb, from);
-	if (ret == -ENOTBLK)
-		ret = generic_file_write_iter(iocb, from);
-	return ret;
+	return gfs2_file_buffered_write(iocb, from);
 }
 
 static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
-- 
2.14.3

Re: [PATCH 10/10] gfs2: Implement buffered iomap write support (2)

[Andreas Gruenbacher]

On 11 January 2018 at 22:15, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> Instead of falling back to generic_file_write_iter when writing to a
> stuffed file that stays stuffed, implement that case separately.  We
> eventually want to get rid of the remaining users of gfs2_write_begin +
> gfs2_write_end so that those functions can eventually be removed, and
> generic_file_write_iter uses that interface.

Oops, please ignore this patch as well.

Thanks,
Andreas

[PATCH 10/10] gfs2: Implement iomap buffered write support (2)

[Andreas Gruenbacher]

Instead of falling back to generic_file_write_iter when writing to a
stuffed file that stays stuffed, implement that case separately.  We
eventually want to get rid of the remaining users of gfs2_write_begin +
gfs2_write_end so that those functions can eventually be removed, and
generic_file_write_iter uses that interface.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/gfs2/aops.h |  1 +
 fs/gfs2/bmap.c | 13 ++++----
 fs/gfs2/bmap.h |  1 +
 fs/gfs2/file.c | 10 ++++---
 5 files changed, 108 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index a4ce16b362bb..852c9b5ecef2 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -22,6 +22,7 @@
 #include <linux/backing-dev.h>
 #include <linux/uio.h>
 #include <trace/events/writeback.h>
+#include <linux/sched/signal.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -847,6 +848,99 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
 	return copied;
 }
 
+ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	loff_t pos = iocb->ki_pos;
+	struct page *page = NULL;
+	ssize_t written = 0, ret;
+
+	BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
+	BUG_ON(!gfs2_is_stuffed(ip));
+
+	ret = gfs2_trans_begin(sdp, RES_DINODE, 0);
+	if (ret)
+		return ret;
+
+	do {
+		struct buffer_head *dibh;
+		unsigned long offset;
+		unsigned long bytes;
+		size_t copied;
+
+		offset = pos & (PAGE_SIZE - 1);
+		bytes = min_t(unsigned long, PAGE_SIZE - offset,
+					     iov_iter_count(from));
+again:
+		/*
+		 * Bring in the user page that we will copy from _first_.
+		 * Otherwise there's a nasty deadlock on copying from the
+		 * same page as we're writing to, without it being marked
+		 * up-to-date.
+		 *
+		 * Not only is this an optimisation, but it is also required
+		 * to check that the address is actually valid, when atomic
+		 * usercopies are used, below.
+		 */
+		if (unlikely(iov_iter_fault_in_readable(from, bytes))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		if (fatal_signal_pending(current)) {
+			ret = -EINTR;
+			goto out;
+		}
+
+		page = grab_cache_page_write_begin(inode->i_mapping, pos >> PAGE_SHIFT, AOP_FLAG_NOFS);
+		if (!page)
+			return -ENOMEM;
+
+		if (!PageUptodate(page)) {
+			ret = stuffed_readpage(ip, page);
+			if (ret)
+				goto out;
+		}
+
+		if (mapping_writably_mapped(inode->i_mapping))
+			flush_dcache_page(page);
+
+		copied = iov_iter_copy_from_user_atomic(page, from, pos, bytes);
+
+		flush_dcache_page(page);
+
+		ret = gfs2_meta_inode_buffer(ip, &dibh);
+		if (ret)
+			goto out;
+		ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+		brelse(dibh);
+		if (ret)
+			goto out;
+
+		unlock_page(page);
+		put_page(page);
+		page = NULL;
+
+		iov_iter_advance(from, copied);
+		if (unlikely(copied == 0)) {
+			bytes = iov_iter_single_seg_count(from);
+			goto again;
+		}
+		pos += copied;
+		written += copied;
+	} while (iov_iter_count(from));
+
+out:
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
+	gfs2_trans_end(sdp);
+	return written ? written : ret;
+}
+
 /**
  * gfs2_write_end
  * @file: The file to write to
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
index 9a2fa61d8ca4..600dbe14c4b7 100644
--- a/fs/gfs2/aops.h
+++ b/fs/gfs2/aops.h
@@ -11,6 +11,7 @@
 
 #include "incore.h"
 
+extern ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from);
 extern void adjust_fs_space(struct inode *inode);
 extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
 				   unsigned int from, unsigned int len);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index e070cb9a8094..40099a18acdf 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -826,7 +826,7 @@ static int gfs2_write_lock(struct inode *inode)
 	return error;
 }
 
-static void gfs2_write_unlock(struct inode *inode)
+void gfs2_write_unlock(struct inode *inode)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -855,14 +855,14 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 
 	if (gfs2_is_stuffed(ip)) {
 		if (pos + length <= gfs2_max_stuffed_size(ip)) {
-			ret = -ENOTBLK;
-			goto out_unlock;
+			/* Keep the inode locked! */
+			return -ENOTBLK;
 		}
 	}
 
 	ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
 	if (ret)
-		goto out_release;
+		goto out_unlock;
 	alloc_required = iomap->type != IOMAP_MAPPED;
 
 	if (alloc_required || gfs2_is_jdata(ip))
@@ -872,7 +872,7 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 		struct gfs2_alloc_parms ap = { .target = data_blocks + ind_blocks };
 		ret = gfs2_quota_lock_check(ip, &ap);
 		if (ret)
-			goto out_release;
+			goto out_unlock;
 
 		ret = gfs2_inplace_reserve(ip, &ap);
 		if (ret)
@@ -923,9 +923,8 @@ static int gfs2_iomap_write_begin(struct inode *inode, loff_t pos, loff_t length
 out_qunlock:
 		gfs2_quota_unlock(ip);
 	}
-out_release:
-	release_metapath(&mp);
 out_unlock:
+	release_metapath(&mp);
 	gfs2_write_unlock(inode);
 	return ret;
 }
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index 5d563c29cb0a..7aa4cf56a6b2 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -62,5 +62,6 @@ extern int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset,
 extern int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd);
 extern void gfs2_free_journal_extents(struct gfs2_jdesc *jd);
 extern int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length);
+extern void gfs2_write_unlock(struct inode *inode);
 
 #endif /* __BMAP_DOT_H__ */
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 94d44e30d188..020c6d8abaf3 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -31,6 +31,7 @@
 #include "gfs2.h"
 #include "incore.h"
 #include "bmap.h"
+#include "aops.h"
 #include "dir.h"
 #include "glock.h"
 #include "glops.h"
@@ -710,6 +711,10 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *fro
 	current->backing_dev_info = inode_to_bdi(inode);
 
 	ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+	if (ret == -ENOTBLK) {
+		ret = gfs2_stuffed_write(iocb, from);
+		gfs2_write_unlock(inode);
+	}
 
 	current->backing_dev_info = NULL;
 
@@ -761,10 +766,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	if (iocb->ki_flags & IOCB_DIRECT)
 		return generic_file_write_iter(iocb, from);
-	ret = gfs2_file_buffered_write(iocb, from);
-	if (ret == -ENOTBLK)
-		ret = generic_file_write_iter(iocb, from);
-	return ret;
+	return gfs2_file_buffered_write(iocb, from);
 }
 
 static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
-- 
2.14.3

Re: [Cluster-devel] [PATCH 00/10] gfs2 iomap buffered write support

[Steven Whitehouse]

Hi,


On 11/01/18 21:14, Andreas Gruenbacher wrote:
> Hello,
>
> this patch queue converts gfs2 to use iomap for buffered writes, which
> uses multi-page block allocations for large writes instead of requiring
> a separate allocation for each page of data.
>
> The patches apply on top of the gfs2 punch-hole patch queue [*].
>
> So far, the only user of iomap_file_buffered_write was xfs, which
> doesn't do data journaling.  To support gfs2's data journaling, patch
> 08/10 adds a new iomap_written iomap operation: if defined, this
> operation is called by iomap_file_buffered_write whenever a page has
> been written to.
>
> This patch queue doesn't convert direct I/O, so we still have a
> remaining user of the old gfs2_write_begin / gfs2_write_end interface
> left.  Once direct I/O has been converted to use iomap, we'll get rid of
> that code, though.
>
> Thanks,
> Andreas
>
> [*] https://www.redhat.com/archives/cluster-devel/2017-December/msg00089.html
>
> Andreas Gruenbacher (10):
>    gfs2: Typo fixes
>    gfs2: Add gfs2_max_stuffed_size
>    gfs2: Minor gfs2_page_add_databufs cleanup
>    gfs2: gfs2_stuffed_write_end cleanup
>    gfs2: gfs2_stuffed_write_end cleanup (fixup)
>    gfs2: Remove ordered write mode handling from gfs2_trans_add_data
>    gfs2: Iomap cleanups and improvements
>    iomap: New iomap_written operation
>    gfs2: Implement iomap buffered write support (1)
>    gfs2: Implement iomap buffered write support (2)
It looks like patches 1 to 3 inclusive could be sent right away as they 
are worthwhile clean ups in their own right - no need to delay those. 
Patches 4 and 5 should be merged into one I think. Otherwise it looks 
like everything is going in the right direction generally, the other 
comments which have been made notwithstanding,

Steve.

>
>   fs/gfs2/aops.c        | 178 ++++++++++++++++------
>   fs/gfs2/aops.h        |  19 +++
>   fs/gfs2/bmap.c        | 407 +++++++++++++++++++++++++++++++++++++-------------
>   fs/gfs2/bmap.h        |   5 +-
>   fs/gfs2/dir.c         |   3 +-
>   fs/gfs2/file.c        |  49 +++++-
>   fs/gfs2/incore.h      |   5 +
>   fs/gfs2/inode.c       |  10 +-
>   fs/gfs2/log.h         |   7 +-
>   fs/gfs2/quota.c       |   5 +-
>   fs/gfs2/trans.c       |  27 +---
>   fs/iomap.c            |  21 ++-
>   include/linux/iomap.h |   9 ++
>   13 files changed, 558 insertions(+), 187 deletions(-)
>   create mode 100644 fs/gfs2/aops.h
>