GFS2: directly write blocks past i_size

GFS2 was relying on the writepage code to write out the zeroed data for fallocate. However, with FALLOC_FL_KEEP_SIZE set, this may be past i_size. If it is, it will be ignored. To work around this, gfs2 now calls write_dirty_buffer directly on the buffer_heads when FALLOC_FL_KEEP_SIZE is set, and it's writing past i_size.

Signed-off-by Benjamin Marzinski <[email protected]>

---
 fs/gfs2/file.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 10 deletions(-)

Index: gfs2-2.6-nmw/fs/gfs2/file.c
===================================================================

fs/gfs2/file.c +20 -6

@@ -617,18 +617,55 @@ static ssize_t gfs2_file_aio_write(struc
 	return generic_file_aio_write(iocb, iov, nr_segs, pos);
 }
 
-static void empty_write_end(struct page *page, unsigned from,
-			   unsigned to)
+static int empty_write_end(struct page *page, unsigned from,
+			   unsigned to, int mode)
 {
-	struct gfs2_inode *ip = GFS2_I(page->mapping->host);
+	struct inode *inode = page->mapping->host;
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct buffer_head *bh;
+	int waiting = 0;
+	unsigned offset, blksize = 1 << inode->i_blkbits;
+	loff_t i_size = i_size_read(inode);
+	pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
 
 	zero_user(page, from, to-from);
 	mark_page_accessed(page);
 
-	if (!gfs2_is_writeback(ip))
-		gfs2_page_add_databufs(ip, page, from, to);
+ 	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);
+		block_commit_write(page, from, to);
+		return 0;
+	}
+
+second_pass:
+	offset = 0;
+	bh = page_buffers(page);
+	while (offset < from) {

fs/gfs2/file.c +10 -4

+	return 0;
 }
 
 static int needs_empty_write(sector_t block, struct inode *inode)

@@ -643,7 +680,8 @@ static int needs_empty_write(sector_t bl
 	return !buffer_mapped(&bh_map);
 }
 
-static int write_empty_blocks(struct page *page, unsigned from, unsigned to)
+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;

@@ -668,7 +706,9 @@ static int write_empty_blocks(struct pag
 							  gfs2_block_map);
 				if (unlikely(ret))
 					return ret;
-				empty_write_end(page, start, end);
+				ret = empty_write_end(page, start, end, mode);
+				if (unlikely(ret))
+					return ret;
 				end = 0;
 			}
 			start = next;

@@ -682,7 +722,9 @@ static int write_empty_blocks(struct pag
 		ret = __block_write_begin(page, start, end - start, gfs2_block_map);
 		if (unlikely(ret))
 			return ret;
-		empty_write_end(page, start, end);
+		ret = empty_write_end(page, start, end, mode);
+		if (unlikely(ret))
+			return ret;
 	}
 
 	return 0;

@@ -731,7 +773,7 @@ static int fallocate_chunk(struct inode 
 
 		if (curr == end)
 			to = end_offset;
-		error = write_empty_blocks(page, from, to);
+		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);

Hi,

Quoted 11 lines · Benjamin Marzinski

GFS2 was relying on the writepage code to write out the zeroed data for fallocate. However, with FALLOC_FL_KEEP_SIZE set, this may be past i_size. If it is, it will be ignored. To work around this, gfs2 now calls write_dirty_buffer directly on the buffer_heads when FALLOC_FL_KEEP_SIZE is set, and it's writing past i_size.

Signed-off-by Benjamin Marzinski <[email protected]>

---
 fs/gfs2/file.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 10 deletions(-)

Generally looks good, but a few minor points....

Quoted 40 lines

Index: gfs2-2.6-nmw/fs/gfs2/file.c
===================================================================

fs/gfs2/file.c +20 -6

@@ -617,18 +617,55 @@ static ssize_t gfs2_file_aio_write(struc
 	return generic_file_aio_write(iocb, iov, nr_segs, pos);
 }
 
-static void empty_write_end(struct page *page, unsigned from,
-			   unsigned to)
+static int empty_write_end(struct page *page, unsigned from,
+			   unsigned to, int mode)
 {
-	struct gfs2_inode *ip = GFS2_I(page->mapping->host);
+	struct inode *inode = page->mapping->host;
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct buffer_head *bh;
+	int waiting = 0;
+	unsigned offset, blksize = 1 << inode->i_blkbits;
+	loff_t i_size = i_size_read(inode);
+	pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
 
 	zero_user(page, from, to-from);
 	mark_page_accessed(page);
 
-	if (!gfs2_is_writeback(ip))
-		gfs2_page_add_databufs(ip, page, from, to);
+ 	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);
+		block_commit_write(page, from, to);
+		return 0;
+	}
+
+second_pass:
+	offset = 0;
+	bh = page_buffers(page);
+	while (offset < from) {

fs/gfs2/file.c static ssize_t gfs2_file_aio_write(struc · line 655

Quoted 10 lines

fs/gfs2/file.c +10 -0

+		offset += blksize;
+		bh = bh->b_this_page;
+	}
+	while (offset < to) {
+		if (waiting){
+			wait_on_buffer(bh);
+			if (!buffer_uptodate(bh))
+				return -EIO;
+		}
+		else {

The else should start on the line above to make it more obvious.

fs/gfs2/file.c static ssize_t gfs2_file_aio_write(struc · line 659

Quoted 4 lines

fs/gfs2/file.c +4 -0

+			set_buffer_uptodate(bh);
+			mark_buffer_dirty(bh);
+			clear_buffer_new(bh);
+			write_dirty_buffer(bh, WRITE);

Should this be WRITE_SYNC or WRITE_SYNC_PLUG I wonder?

fs/gfs2/file.c static ssize_t gfs2_file_aio_write(struc · line 667

Quoted 8 lines

fs/gfs2/file.c +8 -0

+		}
+		offset += blksize;
+		bh = bh->b_this_page;
+	}
+	if (!waiting) {
+		waiting = 1;
+		goto second_pass;
+	}

I think the code might be a bit cleaner if it was just written as two loops, one after the other since most of the loop content seems to be different according to weather "waiting" is set or not.

Otherwise I think this is a good solution,

Steve.

Quoted 46 lines

fs/gfs2/file.c +10 -4

+	return 0;
 }
 
 static int needs_empty_write(sector_t block, struct inode *inode)

@@ -643,7 +680,8 @@ static int needs_empty_write(sector_t bl
 	return !buffer_mapped(&bh_map);
 }
 
-static int write_empty_blocks(struct page *page, unsigned from, unsigned to)
+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;

@@ -668,7 +706,9 @@ static int write_empty_blocks(struct pag
 							  gfs2_block_map);
 				if (unlikely(ret))
 					return ret;
-				empty_write_end(page, start, end);
+				ret = empty_write_end(page, start, end, mode);
+				if (unlikely(ret))
+					return ret;
 				end = 0;
 			}
 			start = next;

@@ -682,7 +722,9 @@ static int write_empty_blocks(struct pag
 		ret = __block_write_begin(page, start, end - start, gfs2_block_map);
 		if (unlikely(ret))
 			return ret;
-		empty_write_end(page, start, end);
+		ret = empty_write_end(page, start, end, mode);
+		if (unlikely(ret))
+			return ret;
 	}
 
 	return 0;

@@ -731,7 +773,7 @@ static int fallocate_chunk(struct inode 
 
 		if (curr == end)
 			to = end_offset;
-		error = write_empty_blocks(page, from, to);
+		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);

Should this be WRITE_SYNC or WRITE_SYNC_PLUG I wonder?

fs/gfs2/file.c static ssize_t gfs2_file_aio_write(struc · line 667

Quoted 8 lines

fs/gfs2/file.c +8 -0

+		}
+		offset += blksize;
+		bh = bh->b_this_page;
+	}
+	if (!waiting) {
+		waiting = 1;
+		goto second_pass;
+	}

I think the code might be a bit cleaner if it was just written as two loops, one after the other since most of the loop content seems to be different according to weather "waiting" is set or not.

Otherwise I think this is a good solution,

Steve.

Quoted 46 lines

fs/gfs2/file.c +10 -4

+	return 0;
 }
 
 static int needs_empty_write(sector_t block, struct inode *inode)

@@ -643,7 +680,8 @@ static int needs_empty_write(sector_t bl
 	return !buffer_mapped(&bh_map);
 }
 
-static int write_empty_blocks(struct page *page, unsigned from, unsigned to)
+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;

@@ -668,7 +706,9 @@ static int write_empty_blocks(struct pag
 							  gfs2_block_map);
 				if (unlikely(ret))
 					return ret;
-				empty_write_end(page, start, end);
+				ret = empty_write_end(page, start, end, mode);
+				if (unlikely(ret))
+					return ret;
 				end = 0;
 			}
 			start = next;

@@ -682,7 +722,9 @@ static int write_empty_blocks(struct pag
 		ret = __block_write_begin(page, start, end - start, gfs2_block_map);
 		if (unlikely(ret))
 			return ret;
-		empty_write_end(page, start, end);
+		ret = empty_write_end(page, start, end, mode);
+		if (unlikely(ret))
+			return ret;
 	}
 
 	return 0;

@@ -731,7 +773,7 @@ static int fallocate_chunk(struct inode 
 
 		if (curr == end)
 			to = end_offset;
-		error = write_empty_blocks(page, from, to);
+		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);