[Cluster-devel] [PATCH] GFS2: rewrite fallocate code to write blocks directly

[Cluster-devel] [PATCH] GFS2: rewrite fallocate code to write blocks directly

[Benjamin Marzinski]

GFS2's fallocate code currently goes through the page cache. Since it's only
writing to the end of the file or to holes in it, it doesn't need to, and it
was causing issues on low memory environments. This patch pulls in some of
Steve's block allocation work, and uses it to simply allocate the blocks for
the file, and zero them out at allocation time.  It provides a slight
performance increase, and it dramatically simplifies the code.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 fs/gfs2/bmap.c   |   12 +++
 fs/gfs2/file.c   |  171 +++++++------------------------------------------------
 fs/gfs2/incore.h |    3 
 3 files changed, 39 insertions(+), 147 deletions(-)

Index: gfs2-2.6-nmw/fs/gfs2/file.c
===================================================================
--- gfs2-2.6-nmw.orig/fs/gfs2/file.c
+++ gfs2-2.6-nmw/fs/gfs2/file.c
@@ -669,135 +669,18 @@ static ssize_t gfs2_file_aio_write(struc
 	return generic_file_aio_write(iocb, iov, nr_segs, pos);
 }
 
-static int empty_write_end(struct page *page, unsigned from,
-			   unsigned to, int mode)
-{
-	struct inode *inode = page->mapping->host;
-	struct gfs2_inode *ip = GFS2_I(inode);
-	struct buffer_head *bh;
-	unsigned offset, blksize = 1 << inode->i_blkbits;
-	pgoff_t end_index = i_size_read(inode) >> PAGE_CACHE_SHIFT;
-
-	zero_user(page, from, to-from);
-	mark_page_accessed(page);
-
-	if (page->index < end_index || !(mode & FALLOC_FL_KEEP_SIZE)) {
-		if (!gfs2_is_writeback(ip))
-			gfs2_page_add_databufs(ip, page, from, to);
-
-		block_commit_write(page, from, to);
-		return 0;
-	}
-
-	offset = 0;
-	bh = page_buffers(page);
-	while (offset < to) {
-		if (offset >= from) {
-			set_buffer_uptodate(bh);
-			mark_buffer_dirty(bh);
-			clear_buffer_new(bh);
-			write_dirty_buffer(bh, WRITE);
-		}
-		offset += blksize;
-		bh = bh->b_this_page;
-	}
-
-	offset = 0;
-	bh = page_buffers(page);
-	while (offset < to) {
-		if (offset >= from) {
-			wait_on_buffer(bh);
-			if (!buffer_uptodate(bh))
-				return -EIO;
-		}
-		offset += blksize;
-		bh = bh->b_this_page;
-	}
-	return 0;
-}
-
-static int needs_empty_write(sector_t block, struct inode *inode)
-{
-	int error;
-	struct buffer_head bh_map = { .b_state = 0, .b_blocknr = 0 };
-
-	bh_map.b_size = 1 << inode->i_blkbits;
-	error = gfs2_block_map(inode, block, &bh_map, 0);
-	if (unlikely(error))
-		return error;
-	return !buffer_mapped(&bh_map);
-}
-
-static int write_empty_blocks(struct page *page, unsigned from, unsigned to,
-			      int mode)
-{
-	struct inode *inode = page->mapping->host;
-	unsigned start, end, next, blksize;
-	sector_t block = page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
-	int ret;
-
-	blksize = 1 << inode->i_blkbits;
-	next = end = 0;
-	while (next < from) {
-		next += blksize;
-		block++;
-	}
-	start = next;
-	do {
-		next += blksize;
-		ret = needs_empty_write(block, inode);
-		if (unlikely(ret < 0))
-			return ret;
-		if (ret == 0) {
-			if (end) {
-				ret = __block_write_begin(page, start, end - start,
-							  gfs2_block_map);
-				if (unlikely(ret))
-					return ret;
-				ret = empty_write_end(page, start, end, mode);
-				if (unlikely(ret))
-					return ret;
-				end = 0;
-			}
-			start = next;
-		}
-		else
-			end = next;
-		block++;
-	} while (next < to);
-
-	if (end) {
-		ret = __block_write_begin(page, start, end - start, gfs2_block_map);
-		if (unlikely(ret))
-			return ret;
-		ret = empty_write_end(page, start, end, mode);
-		if (unlikely(ret))
-			return ret;
-	}
-
-	return 0;
-}
-
 static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
 			   int mode)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct buffer_head *dibh;
 	int error;
-	u64 start = offset >> PAGE_CACHE_SHIFT;
-	unsigned int start_offset = offset & ~PAGE_CACHE_MASK;
-	u64 end = (offset + len - 1) >> PAGE_CACHE_SHIFT;
-	pgoff_t curr;
-	struct page *page;
-	unsigned int end_offset = (offset + len) & ~PAGE_CACHE_MASK;
-	unsigned int from, to;
-
-	if (!end_offset)
-		end_offset = PAGE_CACHE_SIZE;
+	unsigned int nr_blks;
+	sector_t lblock = offset >> inode->i_blkbits;
 
 	error = gfs2_meta_inode_buffer(ip, &dibh);
 	if (unlikely(error))
-		goto out;
+		return error;
 
 	gfs2_trans_add_bh(ip->i_gl, dibh, 1);
 
@@ -807,39 +690,31 @@ static int fallocate_chunk(struct inode 
 			goto out;
 	}
 
-	curr = start;
-	offset = start << PAGE_CACHE_SHIFT;
-	from = start_offset;
-	to = PAGE_CACHE_SIZE;
-	while (curr <= end) {
-		page = grab_cache_page_write_begin(inode->i_mapping, curr,
-						   AOP_FLAG_NOFS);
-		if (unlikely(!page)) {
-			error = -ENOMEM;
-			goto out;
-		}
+	while (len) {
+		struct buffer_head bh_map = { .b_state = 0, .b_blocknr = 0 };
+		bh_map.b_size = len;
+		set_buffer_zeronew(&bh_map);
 
-		if (curr == end)
-			to = end_offset;
-		error = write_empty_blocks(page, from, to, mode);
-		if (!error && offset + to > inode->i_size &&
-		    !(mode & FALLOC_FL_KEEP_SIZE)) {
-			i_size_write(inode, offset + to);
-		}
-		unlock_page(page);
-		page_cache_release(page);
-		if (error)
+		error = gfs2_block_map(inode, lblock, &bh_map, 1);
+		if (unlikely(error))
 			goto out;
-		curr++;
-		offset += PAGE_CACHE_SIZE;
-		from = 0;
+		len -= bh_map.b_size;
+		nr_blks = bh_map.b_size >> inode->i_blkbits;
+		lblock += nr_blks;
+		if (!buffer_new(&bh_map))
+			continue;
+		if (unlikely(!buffer_zeronew(&bh_map))) {
+			error = -EIO;
+			goto out;
+		}
 	}
+	if (offset + len > inode->i_size && !(mode & FALLOC_FL_KEEP_SIZE))
+		i_size_write(inode, offset + len);
 
 	mark_inode_dirty(inode);
 
-	brelse(dibh);
-
 out:
+	brelse(dibh);
 	return error;
 }
 
@@ -879,6 +754,7 @@ static long gfs2_fallocate(struct file *
 	int error;
 	loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
 	loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
+	loff_t max_chunk_size = UINT_MAX & bsize_mask;
 	next = (next + 1) << sdp->sd_sb.sb_bsize_shift;
 
 	/* We only support the FALLOC_FL_KEEP_SIZE mode */
@@ -932,7 +808,8 @@ retry:
 			goto out_qunlock;
 		}
 		max_bytes = bytes;
-		calc_max_reserv(ip, len, &max_bytes, &data_blocks, &ind_blocks);
+		calc_max_reserv(ip, (len > max_chunk_size)? max_chunk_size: len,
+				&max_bytes, &data_blocks, &ind_blocks);
 		al->al_requested = data_blocks + ind_blocks;
 
 		rblocks = RES_DINODE + ind_blocks + RES_STATFS + RES_QUOTA +
Index: gfs2-2.6-nmw/fs/gfs2/bmap.c
===================================================================
--- gfs2-2.6-nmw.orig/fs/gfs2/bmap.c
+++ gfs2-2.6-nmw/fs/gfs2/bmap.c
@@ -10,6 +10,7 @@
 #include <linux/spinlock.h>
 #include <linux/completion.h>
 #include <linux/buffer_head.h>
+#include <linux/blkdev.h>
 #include <linux/gfs2_ondisk.h>
 #include <linux/crc32.h>
 
@@ -427,12 +428,14 @@ static int gfs2_bmap_alloc(struct inode 
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	struct super_block *sb = sdp->sd_vfs;
 	struct buffer_head *dibh = mp->mp_bh[0];
 	u64 bn, dblock = 0;
 	unsigned n, i, blks, alloced = 0, iblks = 0, branch_start = 0;
 	unsigned dblks = 0;
 	unsigned ptrs_per_blk;
 	const unsigned end_of_metadata = height - 1;
+	int ret;
 	int eob = 0;
 	enum alloc_state state;
 	__be64 *ptr;
@@ -535,6 +538,15 @@ static int gfs2_bmap_alloc(struct inode 
 			dblock = bn;
 			while (n-- > 0)
 				*ptr++ = cpu_to_be64(bn++);
+			if (buffer_zeronew(bh_map)) {
+				ret = sb_issue_zeroout(sb, dblock, dblks,
+						       GFP_NOFS);
+				if (ret) {
+					fs_err(sdp,
+					       "Failed to zero data buffers\n");
+					clear_buffer_zeronew(bh_map);
+				}
+			}
 			break;
 		}
 	} while ((state != ALLOC_DATA) || !dblock);
Index: gfs2-2.6-nmw/fs/gfs2/incore.h
===================================================================
--- gfs2-2.6-nmw.orig/fs/gfs2/incore.h
+++ gfs2-2.6-nmw/fs/gfs2/incore.h
@@ -103,12 +103,15 @@ struct gfs2_rgrpd {
 enum gfs2_state_bits {
 	BH_Pinned = BH_PrivateStart,
 	BH_Escaped = BH_PrivateStart + 1,
+	BH_Zeronew = BH_PrivateStart + 2,
 };
 
 BUFFER_FNS(Pinned, pinned)
 TAS_BUFFER_FNS(Pinned, pinned)
 BUFFER_FNS(Escaped, escaped)
 TAS_BUFFER_FNS(Escaped, escaped)
+BUFFER_FNS(Zeronew, zeronew)
+TAS_BUFFER_FNS(Zeronew, zeronew)
 
 struct gfs2_bufdata {
 	struct buffer_head *bd_bh;

[Cluster-devel] [PATCH] GFS2: rewrite fallocate code to write blocks directly

[Steven Whitehouse]

Hi,

This looks really good. Sorry for not spotting it first time around - I
think it got lost in the backlog when I came back from holiday. Its now
in the -nmw tree. Thanks,

Steve.

On Mon, 2011-09-12 at 18:15 -0500, Benjamin Marzinski wrote:
> GFS2's fallocate code currently goes through the page cache. Since it's only
> writing to the end of the file or to holes in it, it doesn't need to, and it
> was causing issues on low memory environments. This patch pulls in some of
> Steve's block allocation work, and uses it to simply allocate the blocks for
> the file, and zero them out at allocation time.  It provides a slight
> performance increase, and it dramatically simplifies the code.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  fs/gfs2/bmap.c   |   12 +++
>  fs/gfs2/file.c   |  171 +++++++------------------------------------------------
>  fs/gfs2/incore.h |    3 
>  3 files changed, 39 insertions(+), 147 deletions(-)
> 
> Index: gfs2-2.6-nmw/fs/gfs2/file.c
> ===================================================================
> --- gfs2-2.6-nmw.orig/fs/gfs2/file.c
> +++ gfs2-2.6-nmw/fs/gfs2/file.c
> @@ -669,135 +669,18 @@ static ssize_t gfs2_file_aio_write(struc
>  	return generic_file_aio_write(iocb, iov, nr_segs, pos);
>  }
>  
> -static int empty_write_end(struct page *page, unsigned from,
> -			   unsigned to, int mode)
> -{
> -	struct inode *inode = page->mapping->host;
> -	struct gfs2_inode *ip = GFS2_I(inode);
> -	struct buffer_head *bh;
> -	unsigned offset, blksize = 1 << inode->i_blkbits;
> -	pgoff_t end_index = i_size_read(inode) >> PAGE_CACHE_SHIFT;
> -
> -	zero_user(page, from, to-from);
> -	mark_page_accessed(page);
> -
> -	if (page->index < end_index || !(mode & FALLOC_FL_KEEP_SIZE)) {
> -		if (!gfs2_is_writeback(ip))
> -			gfs2_page_add_databufs(ip, page, from, to);
> -
> -		block_commit_write(page, from, to);
> -		return 0;
> -	}
> -
> -	offset = 0;
> -	bh = page_buffers(page);
> -	while (offset < to) {
> -		if (offset >= from) {
> -			set_buffer_uptodate(bh);
> -			mark_buffer_dirty(bh);
> -			clear_buffer_new(bh);
> -			write_dirty_buffer(bh, WRITE);
> -		}
> -		offset += blksize;
> -		bh = bh->b_this_page;
> -	}
> -
> -	offset = 0;
> -	bh = page_buffers(page);
> -	while (offset < to) {
> -		if (offset >= from) {
> -			wait_on_buffer(bh);
> -			if (!buffer_uptodate(bh))
> -				return -EIO;
> -		}
> -		offset += blksize;
> -		bh = bh->b_this_page;
> -	}
> -	return 0;
> -}
> -
> -static int needs_empty_write(sector_t block, struct inode *inode)
> -{
> -	int error;
> -	struct buffer_head bh_map = { .b_state = 0, .b_blocknr = 0 };
> -
> -	bh_map.b_size = 1 << inode->i_blkbits;
> -	error = gfs2_block_map(inode, block, &bh_map, 0);
> -	if (unlikely(error))
> -		return error;
> -	return !buffer_mapped(&bh_map);
> -}
> -
> -static int write_empty_blocks(struct page *page, unsigned from, unsigned to,
> -			      int mode)
> -{
> -	struct inode *inode = page->mapping->host;
> -	unsigned start, end, next, blksize;
> -	sector_t block = page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
> -	int ret;
> -
> -	blksize = 1 << inode->i_blkbits;
> -	next = end = 0;
> -	while (next < from) {
> -		next += blksize;
> -		block++;
> -	}
> -	start = next;
> -	do {
> -		next += blksize;
> -		ret = needs_empty_write(block, inode);
> -		if (unlikely(ret < 0))
> -			return ret;
> -		if (ret == 0) {
> -			if (end) {
> -				ret = __block_write_begin(page, start, end - start,
> -							  gfs2_block_map);
> -				if (unlikely(ret))
> -					return ret;
> -				ret = empty_write_end(page, start, end, mode);
> -				if (unlikely(ret))
> -					return ret;
> -				end = 0;
> -			}
> -			start = next;
> -		}
> -		else
> -			end = next;
> -		block++;
> -	} while (next < to);
> -
> -	if (end) {
> -		ret = __block_write_begin(page, start, end - start, gfs2_block_map);
> -		if (unlikely(ret))
> -			return ret;
> -		ret = empty_write_end(page, start, end, mode);
> -		if (unlikely(ret))
> -			return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
>  			   int mode)
>  {
>  	struct gfs2_inode *ip = GFS2_I(inode);
>  	struct buffer_head *dibh;
>  	int error;
> -	u64 start = offset >> PAGE_CACHE_SHIFT;
> -	unsigned int start_offset = offset & ~PAGE_CACHE_MASK;
> -	u64 end = (offset + len - 1) >> PAGE_CACHE_SHIFT;
> -	pgoff_t curr;
> -	struct page *page;
> -	unsigned int end_offset = (offset + len) & ~PAGE_CACHE_MASK;
> -	unsigned int from, to;
> -
> -	if (!end_offset)
> -		end_offset = PAGE_CACHE_SIZE;
> +	unsigned int nr_blks;
> +	sector_t lblock = offset >> inode->i_blkbits;
>  
>  	error = gfs2_meta_inode_buffer(ip, &dibh);
>  	if (unlikely(error))
> -		goto out;
> +		return error;
>  
>  	gfs2_trans_add_bh(ip->i_gl, dibh, 1);
>  
> @@ -807,39 +690,31 @@ static int fallocate_chunk(struct inode 
>  			goto out;
>  	}
>  
> -	curr = start;
> -	offset = start << PAGE_CACHE_SHIFT;
> -	from = start_offset;
> -	to = PAGE_CACHE_SIZE;
> -	while (curr <= end) {
> -		page = grab_cache_page_write_begin(inode->i_mapping, curr,
> -						   AOP_FLAG_NOFS);
> -		if (unlikely(!page)) {
> -			error = -ENOMEM;
> -			goto out;
> -		}
> +	while (len) {
> +		struct buffer_head bh_map = { .b_state = 0, .b_blocknr = 0 };
> +		bh_map.b_size = len;
> +		set_buffer_zeronew(&bh_map);
>  
> -		if (curr == end)
> -			to = end_offset;
> -		error = write_empty_blocks(page, from, to, mode);
> -		if (!error && offset + to > inode->i_size &&
> -		    !(mode & FALLOC_FL_KEEP_SIZE)) {
> -			i_size_write(inode, offset + to);
> -		}
> -		unlock_page(page);
> -		page_cache_release(page);
> -		if (error)
> +		error = gfs2_block_map(inode, lblock, &bh_map, 1);
> +		if (unlikely(error))
>  			goto out;
> -		curr++;
> -		offset += PAGE_CACHE_SIZE;
> -		from = 0;
> +		len -= bh_map.b_size;
> +		nr_blks = bh_map.b_size >> inode->i_blkbits;
> +		lblock += nr_blks;
> +		if (!buffer_new(&bh_map))
> +			continue;
> +		if (unlikely(!buffer_zeronew(&bh_map))) {
> +			error = -EIO;
> +			goto out;
> +		}
>  	}
> +	if (offset + len > inode->i_size && !(mode & FALLOC_FL_KEEP_SIZE))
> +		i_size_write(inode, offset + len);
>  
>  	mark_inode_dirty(inode);
>  
> -	brelse(dibh);
> -
>  out:
> +	brelse(dibh);
>  	return error;
>  }
>  
> @@ -879,6 +754,7 @@ static long gfs2_fallocate(struct file *
>  	int error;
>  	loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
>  	loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
> +	loff_t max_chunk_size = UINT_MAX & bsize_mask;
>  	next = (next + 1) << sdp->sd_sb.sb_bsize_shift;
>  
>  	/* We only support the FALLOC_FL_KEEP_SIZE mode */
> @@ -932,7 +808,8 @@ retry:
>  			goto out_qunlock;
>  		}
>  		max_bytes = bytes;
> -		calc_max_reserv(ip, len, &max_bytes, &data_blocks, &ind_blocks);
> +		calc_max_reserv(ip, (len > max_chunk_size)? max_chunk_size: len,
> +				&max_bytes, &data_blocks, &ind_blocks);
>  		al->al_requested = data_blocks + ind_blocks;
>  
>  		rblocks = RES_DINODE + ind_blocks + RES_STATFS + RES_QUOTA +
> Index: gfs2-2.6-nmw/fs/gfs2/bmap.c
> ===================================================================
> --- gfs2-2.6-nmw.orig/fs/gfs2/bmap.c
> +++ gfs2-2.6-nmw/fs/gfs2/bmap.c
> @@ -10,6 +10,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/completion.h>
>  #include <linux/buffer_head.h>
> +#include <linux/blkdev.h>
>  #include <linux/gfs2_ondisk.h>
>  #include <linux/crc32.h>
>  
> @@ -427,12 +428,14 @@ static int gfs2_bmap_alloc(struct inode 
>  {
>  	struct gfs2_inode *ip = GFS2_I(inode);
>  	struct gfs2_sbd *sdp = GFS2_SB(inode);
> +	struct super_block *sb = sdp->sd_vfs;
>  	struct buffer_head *dibh = mp->mp_bh[0];
>  	u64 bn, dblock = 0;
>  	unsigned n, i, blks, alloced = 0, iblks = 0, branch_start = 0;
>  	unsigned dblks = 0;
>  	unsigned ptrs_per_blk;
>  	const unsigned end_of_metadata = height - 1;
> +	int ret;
>  	int eob = 0;
>  	enum alloc_state state;
>  	__be64 *ptr;
> @@ -535,6 +538,15 @@ static int gfs2_bmap_alloc(struct inode 
>  			dblock = bn;
>  			while (n-- > 0)
>  				*ptr++ = cpu_to_be64(bn++);
> +			if (buffer_zeronew(bh_map)) {
> +				ret = sb_issue_zeroout(sb, dblock, dblks,
> +						       GFP_NOFS);
> +				if (ret) {
> +					fs_err(sdp,
> +					       "Failed to zero data buffers\n");
> +					clear_buffer_zeronew(bh_map);
> +				}
> +			}
>  			break;
>  		}
>  	} while ((state != ALLOC_DATA) || !dblock);
> Index: gfs2-2.6-nmw/fs/gfs2/incore.h
> ===================================================================
> --- gfs2-2.6-nmw.orig/fs/gfs2/incore.h
> +++ gfs2-2.6-nmw/fs/gfs2/incore.h
> @@ -103,12 +103,15 @@ struct gfs2_rgrpd {
>  enum gfs2_state_bits {
>  	BH_Pinned = BH_PrivateStart,
>  	BH_Escaped = BH_PrivateStart + 1,
> +	BH_Zeronew = BH_PrivateStart + 2,
>  };
>  
>  BUFFER_FNS(Pinned, pinned)
>  TAS_BUFFER_FNS(Pinned, pinned)
>  BUFFER_FNS(Escaped, escaped)
>  TAS_BUFFER_FNS(Escaped, escaped)
> +BUFFER_FNS(Zeronew, zeronew)
> +TAS_BUFFER_FNS(Zeronew, zeronew)
>  
>  struct gfs2_bufdata {
>  	struct buffer_head *bd_bh;
>