[PATCH, RFC] Clean up of delalloc code and map-on-close/rename fixes

[PATCH, RFC] Clean up of delalloc code and map-on-close/rename fixes

[Theodore Ts'o]

Ok, here are the patches I was talking about at our call today.  They
clean up the delalloc code, and then add an ioctl for forcing delayed
allocation blocks to be allocated.  The last two patches force delayed
allocation blocks to be allocated on close if the file had been
previously truncated, and on rename if the old file is displacing
another file.

I would appreicate some review.

Thanks!!

							- Ted

[PATCH, RFC] ext4: Simplify delalloc implementation by removing mpd.get_block

[Theodore Ts'o]

This parameter was always set to ext4_da_get_block_write().

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/inode.c |  125 +++++++++++++++++++++++++-----------------------------
 1 files changed, 58 insertions(+), 67 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index fd08f78..a533314 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1675,7 +1675,6 @@ struct mpage_da_data {
 	struct inode *inode;
 	struct buffer_head lbh;			/* extent of blocks */
 	unsigned long first_page, next_page;	/* extent of pages */
-	get_block_t *get_block;
 	struct writeback_control *wbc;
 	int io_done;
 	int pages_written;
@@ -1689,7 +1688,6 @@ struct mpage_da_data {
  * @mpd->inode: inode
  * @mpd->first_page: first page of the extent
  * @mpd->next_page: page after the last page of the extent
- * @mpd->get_block: the filesystem's block mapper function
  *
  * By the time mpage_da_submit_io() is called we expect all blocks
  * to be allocated. this may be wrong if allocation failed.
@@ -1899,16 +1897,60 @@ static void ext4_print_free_blocks(struct inode *inode)
 	return;
 }
 
+#define		EXT4_DELALLOC_RSVED	1
+static int ext4_da_get_block_write(struct inode *inode, sector_t iblock,
+				   struct buffer_head *bh_result, int create)
+{
+	int ret;
+	unsigned max_blocks = bh_result->b_size >> inode->i_blkbits;
+	loff_t disksize = EXT4_I(inode)->i_disksize;
+	handle_t *handle = NULL;
+
+	handle = ext4_journal_current_handle();
+	BUG_ON(!handle);
+	ret = ext4_get_blocks_wrap(handle, inode, iblock, max_blocks,
+				   bh_result, create, 0, EXT4_DELALLOC_RSVED);
+	if (ret <= 0)
+		return ret;
+
+	bh_result->b_size = (ret << inode->i_blkbits);
+
+	if (ext4_should_order_data(inode)) {
+		int retval;
+		retval = ext4_jbd2_file_inode(handle, inode);
+		if (retval)
+			/*
+			 * Failed to add inode for ordered mode. Don't
+			 * update file size
+			 */
+			return retval;
+	}
+
+	/*
+	 * Update on-disk size along with block allocation we don't
+	 * use 'extend_disksize' as size may change within already
+	 * allocated block -bzzz
+	 */
+	disksize = ((loff_t) iblock + ret) << inode->i_blkbits;
+	if (disksize > i_size_read(inode))
+		disksize = i_size_read(inode);
+	if (disksize > EXT4_I(inode)->i_disksize) {
+		ext4_update_i_disksize(inode, disksize);
+		ret = ext4_mark_inode_dirty(handle, inode);
+		return ret;
+	}
+	return 0;
+}
+
 /*
  * mpage_da_map_blocks - go through given space
  *
  * @mpd->lbh - bh describing space
- * @mpd->get_block - the filesystem's block mapper function
  *
  * The function skips space we know is already mapped to disk blocks.
  *
  */
-static int  mpage_da_map_blocks(struct mpage_da_data *mpd)
+static int mpage_da_map_blocks(struct mpage_da_data *mpd)
 {
 	int err = 0;
 	struct buffer_head new;
@@ -1930,30 +1972,29 @@ static int  mpage_da_map_blocks(struct mpage_da_data *mpd)
 	 */
 	if (!new.b_size)
 		return 0;
-	err = mpd->get_block(mpd->inode, next, &new, 1);
-	if (err) {
 
-		/* If get block returns with error
-		 * we simply return. Later writepage
-		 * will redirty the page and writepages
-		 * will find the dirty page again
+	err = ext4_da_get_block_write(mpd->inode, next, &new, 1);
+	if (err) {
+		/*
+		 * If get block returns with error we simply
+		 * return. Later writepage will redirty the page and
+		 * writepages will find the dirty page again
 		 */
 		if (err == -EAGAIN)
 			return 0;
 
 		if (err == -ENOSPC &&
-				ext4_count_free_blocks(mpd->inode->i_sb)) {
+		    ext4_count_free_blocks(mpd->inode->i_sb)) {
 			mpd->retval = err;
 			return 0;
 		}
 
 		/*
-		 * get block failure will cause us
-		 * to loop in writepages. Because
-		 * a_ops->writepage won't be able to
-		 * make progress. The page will be redirtied
-		 * by writepage and writepages will again
-		 * try to write the same.
+		 * get block failure will cause us to loop in
+		 * writepages, because a_ops->writepage won't be able
+		 * to make progress. The page will be redirtied by
+		 * writepage and writepages will again try to write
+		 * the same.
 		 */
 		printk(KERN_EMERG "%s block allocation failed for inode %lu "
 				  "at logical offset %llu with max blocks "
@@ -2182,7 +2223,6 @@ static int __mpage_da_writepage(struct page *page,
  *
  * @mapping: address space structure to write
  * @wbc: subtract the number of written pages from *@wbc->nr_to_write
- * @get_block: the filesystem's block mapper function.
  *
  * This is a library function, which implements the writepages()
  * address_space_operation.
@@ -2193,9 +2233,6 @@ static int mpage_da_writepages(struct address_space *mapping,
 {
 	int ret;
 
-	if (!mpd->get_block)
-		return generic_writepages(mapping, wbc);
-
 	mpd->lbh.b_size = 0;
 	mpd->lbh.b_state = 0;
 	mpd->lbh.b_blocknr = 0;
@@ -2259,51 +2296,6 @@ static int ext4_da_get_block_prep(struct inode *inode, sector_t iblock,
 
 	return ret;
 }
-#define		EXT4_DELALLOC_RSVED	1
-static int ext4_da_get_block_write(struct inode *inode, sector_t iblock,
-				   struct buffer_head *bh_result, int create)
-{
-	int ret;
-	unsigned max_blocks = bh_result->b_size >> inode->i_blkbits;
-	loff_t disksize = EXT4_I(inode)->i_disksize;
-	handle_t *handle = NULL;
-
-	handle = ext4_journal_current_handle();
-	BUG_ON(!handle);
-	ret = ext4_get_blocks_wrap(handle, inode, iblock, max_blocks,
-			bh_result, create, 0, EXT4_DELALLOC_RSVED);
-	if (ret > 0) {
-
-		bh_result->b_size = (ret << inode->i_blkbits);
-
-		if (ext4_should_order_data(inode)) {
-			int retval;
-			retval = ext4_jbd2_file_inode(handle, inode);
-			if (retval)
-				/*
-				 * Failed to add inode for ordered
-				 * mode. Don't update file size
-				 */
-				return retval;
-		}
-
-		/*
-		 * Update on-disk size along with block allocation
-		 * we don't use 'extend_disksize' as size may change
-		 * within already allocated block -bzzz
-		 */
-		disksize = ((loff_t) iblock + ret) << inode->i_blkbits;
-		if (disksize > i_size_read(inode))
-			disksize = i_size_read(inode);
-		if (disksize > EXT4_I(inode)->i_disksize) {
-			ext4_update_i_disksize(inode, disksize);
-			ret = ext4_mark_inode_dirty(handle, inode);
-			return ret;
-		}
-		ret = 0;
-	}
-	return ret;
-}
 
 static int ext4_bh_unmapped_or_delay(handle_t *handle, struct buffer_head *bh)
 {
@@ -2554,7 +2546,6 @@ retry:
 			dump_stack();
 			goto out_writepages;
 		}
-		mpd.get_block = ext4_da_get_block_write;
 		ret = mpage_da_writepages(mapping, wbc, &mpd);
 
 		ext4_journal_stop(handle);
-- 
1.5.6.3

[PATCH, RFC] ext4: Save stack space by removing fake buffer heads

[Theodore Ts'o]

Struct mpage_da_data and mpage_add_bh_to_extent() use a fake struct
buffer_head which is 104 bytes on an x86_64 system, but only use 24
bytes of the structure.  On systems that use a spinlock for atomic_t,
the stack savings will be even greater.

It turns out that using a fake struct buffer_head doesn't even save
that much code, and it makes the code more confusing since it's not
used as a "real" buffer head.  So just store pass b_size and b_state
in mpage_add_bh_to_extent(), and store b_size, b_state, and b_block_nr
in the mpage_da_data structure.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/inode.c |   83 ++++++++++++++++++++++++++-----------------------------
 1 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index a533314..407aa85 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1673,7 +1673,9 @@ static void ext4_da_page_release_reservation(struct page *page,
 
 struct mpage_da_data {
 	struct inode *inode;
-	struct buffer_head lbh;			/* extent of blocks */
+	sector_t b_blocknr;		/* start block number of extent */
+	size_t b_size;			/* size of extent */
+	unsigned long b_state;		/* state of the extent */
 	unsigned long first_page, next_page;	/* extent of pages */
 	struct writeback_control *wbc;
 	int io_done;
@@ -1707,7 +1709,7 @@ static int mpage_da_submit_io(struct mpage_da_data *mpd)
 	/*
 	 * We need to start from the first_page to the next_page - 1
 	 * to make sure we also write the mapped dirty buffer_heads.
-	 * If we look at mpd->lbh.b_blocknr we would only be looking
+	 * If we look at mpd->b_blocknr we would only be looking
 	 * at the currently mapped buffer_heads.
 	 */
 	index = mpd->first_page;
@@ -1945,7 +1947,7 @@ static int ext4_da_get_block_write(struct inode *inode, sector_t iblock,
 /*
  * mpage_da_map_blocks - go through given space
  *
- * @mpd->lbh - bh describing space
+ * @mpd - bh describing space
  *
  * The function skips space we know is already mapped to disk blocks.
  *
@@ -1954,18 +1956,18 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
 {
 	int err = 0;
 	struct buffer_head new;
-	struct buffer_head *lbh = &mpd->lbh;
 	sector_t next;
 
 	/*
 	 * We consider only non-mapped and non-allocated blocks
 	 */
-	if (buffer_mapped(lbh) && !buffer_delay(lbh))
+	if ((mpd->b_state  & (1 << BH_Mapped)) &&
+	    !(mpd->b_state & (1 << BH_Delay)))
 		return 0;
-	new.b_state = lbh->b_state;
+	new.b_state = mpd->b_state;
 	new.b_blocknr = 0;
-	new.b_size = lbh->b_size;
-	next = lbh->b_blocknr;
+	new.b_size = mpd->b_size;
+	next = mpd->b_blocknr;
 	/*
 	 * If we didn't accumulate anything
 	 * to write simply return
@@ -2001,7 +2003,7 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
 				  "%zd with error %d\n",
 				  __func__, mpd->inode->i_ino,
 				  (unsigned long long)next,
-				  lbh->b_size >> mpd->inode->i_blkbits, err);
+				  mpd->b_size >> mpd->inode->i_blkbits, err);
 		printk(KERN_EMERG "This should not happen.!! "
 					"Data will be lost\n");
 		if (err == -ENOSPC) {
@@ -2009,7 +2011,7 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
 		}
 		/* invlaidate all the pages */
 		ext4_da_block_invalidatepages(mpd, next,
-				lbh->b_size >> mpd->inode->i_blkbits);
+				mpd->b_size >> mpd->inode->i_blkbits);
 		return err;
 	}
 	BUG_ON(new.b_size == 0);
@@ -2021,7 +2023,8 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
 	 * If blocks are delayed marked, we need to
 	 * put actual blocknr and drop delayed bit
 	 */
-	if (buffer_delay(lbh) || buffer_unwritten(lbh))
+	if ((mpd->b_state & (1 << BH_Delay)) ||
+	    (mpd->b_state & (1 << BH_Unwritten)))
 		mpage_put_bnr_to_bhs(mpd, next, &new);
 
 	return 0;
@@ -2040,12 +2043,11 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
  * the function is used to collect contig. blocks in same state
  */
 static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
-				   sector_t logical, struct buffer_head *bh)
+				   sector_t logical, size_t b_size,
+				   unsigned long b_state)
 {
 	sector_t next;
-	size_t b_size = bh->b_size;
-	struct buffer_head *lbh = &mpd->lbh;
-	int nrblocks = lbh->b_size >> mpd->inode->i_blkbits;
+	int nrblocks = mpd->b_size >> mpd->inode->i_blkbits;
 
 	/* check if thereserved journal credits might overflow */
 	if (!(EXT4_I(mpd->inode)->i_flags & EXT4_EXTENTS_FL)) {
@@ -2072,19 +2074,19 @@ static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
 	/*
 	 * First block in the extent
 	 */
-	if (lbh->b_size == 0) {
-		lbh->b_blocknr = logical;
-		lbh->b_size = b_size;
-		lbh->b_state = bh->b_state & BH_FLAGS;
+	if (mpd->b_size == 0) {
+		mpd->b_blocknr = logical;
+		mpd->b_size = b_size;
+		mpd->b_state = b_state & BH_FLAGS;
 		return;
 	}
 
-	next = lbh->b_blocknr + nrblocks;
+	next = mpd->b_blocknr + nrblocks;
 	/*
 	 * Can we merge the block to our big extent?
 	 */
-	if (logical == next && (bh->b_state & BH_FLAGS) == lbh->b_state) {
-		lbh->b_size += b_size;
+	if (logical == next && (b_state & BH_FLAGS) == mpd->b_state) {
+		mpd->b_size += b_size;
 		return;
 	}
 
@@ -2113,7 +2115,7 @@ static int __mpage_da_writepage(struct page *page,
 {
 	struct mpage_da_data *mpd = data;
 	struct inode *inode = mpd->inode;
-	struct buffer_head *bh, *head, fake;
+	struct buffer_head *bh, *head;
 	sector_t logical;
 
 	if (mpd->io_done) {
@@ -2155,9 +2157,9 @@ static int __mpage_da_writepage(struct page *page,
 		/*
 		 * ... and blocks
 		 */
-		mpd->lbh.b_size = 0;
-		mpd->lbh.b_state = 0;
-		mpd->lbh.b_blocknr = 0;
+		mpd->b_size = 0;
+		mpd->b_state = 0;
+		mpd->b_blocknr = 0;
 	}
 
 	mpd->next_page = page->index + 1;
@@ -2165,16 +2167,8 @@ static int __mpage_da_writepage(struct page *page,
 		  (PAGE_CACHE_SHIFT - inode->i_blkbits);
 
 	if (!page_has_buffers(page)) {
-		/*
-		 * There is no attached buffer heads yet (mmap?)
-		 * we treat the page asfull of dirty blocks
-		 */
-		bh = &fake;
-		bh->b_size = PAGE_CACHE_SIZE;
-		bh->b_state = 0;
-		set_buffer_dirty(bh);
-		set_buffer_uptodate(bh);
-		mpage_add_bh_to_extent(mpd, logical, bh);
+		mpage_add_bh_to_extent(mpd, logical, PAGE_CACHE_SIZE,
+				       (1 << BH_Dirty) | (1 << BH_Uptodate));
 		if (mpd->io_done)
 			return MPAGE_DA_EXTENT_TAIL;
 	} else {
@@ -2192,8 +2186,10 @@ static int __mpage_da_writepage(struct page *page,
 			 * with the page in ext4_da_writepage
 			 */
 			if (buffer_dirty(bh) &&
-				(!buffer_mapped(bh) || buffer_delay(bh))) {
-				mpage_add_bh_to_extent(mpd, logical, bh);
+			    (!buffer_mapped(bh) || buffer_delay(bh))) {
+				mpage_add_bh_to_extent(mpd, logical,
+						       bh->b_size,
+						       bh->b_state);
 				if (mpd->io_done)
 					return MPAGE_DA_EXTENT_TAIL;
 			} else if (buffer_dirty(bh) && (buffer_mapped(bh))) {
@@ -2205,9 +2201,8 @@ static int __mpage_da_writepage(struct page *page,
 				 * unmapped buffer_head later we need to
 				 * use the b_state flag of that buffer_head.
 				 */
-				if (mpd->lbh.b_size == 0)
-					mpd->lbh.b_state =
-						bh->b_state & BH_FLAGS;
+				if (mpd->b_size == 0)
+					mpd->b_state = bh->b_state & BH_FLAGS;
 			}
 			logical++;
 		} while ((bh = bh->b_this_page) != head);
@@ -2233,9 +2228,9 @@ static int mpage_da_writepages(struct address_space *mapping,
 {
 	int ret;
 
-	mpd->lbh.b_size = 0;
-	mpd->lbh.b_state = 0;
-	mpd->lbh.b_blocknr = 0;
+	mpd->b_size = 0;
+	mpd->b_state = 0;
+	mpd->b_blocknr = 0;
 	mpd->first_page = 0;
 	mpd->next_page = 0;
 	mpd->io_done = 0;
-- 
1.5.6.3

[PATCH, RFC] ext4: Simplify delalloc code by removing mpage_da_writepages()

[Theodore Ts'o]

The mpage_da_writepages() function is only used in one place, so
inline it to simplify the call stack and make the code easier to
understand.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/inode.c |   74 +++++++++++++++++++++++-------------------------------
 1 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 407aa85..1216fb9 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2212,47 +2212,6 @@ static int __mpage_da_writepage(struct page *page,
 }
 
 /*
- * mpage_da_writepages - walk the list of dirty pages of the given
- * address space, allocates non-allocated blocks, maps newly-allocated
- * blocks to existing bhs and issue IO them
- *
- * @mapping: address space structure to write
- * @wbc: subtract the number of written pages from *@wbc->nr_to_write
- *
- * This is a library function, which implements the writepages()
- * address_space_operation.
- */
-static int mpage_da_writepages(struct address_space *mapping,
-			       struct writeback_control *wbc,
-			       struct mpage_da_data *mpd)
-{
-	int ret;
-
-	mpd->b_size = 0;
-	mpd->b_state = 0;
-	mpd->b_blocknr = 0;
-	mpd->first_page = 0;
-	mpd->next_page = 0;
-	mpd->io_done = 0;
-	mpd->pages_written = 0;
-	mpd->retval = 0;
-
-	ret = write_cache_pages(mapping, wbc, __mpage_da_writepage, mpd);
-	/*
-	 * Handle last extent of pages
-	 */
-	if (!mpd->io_done && mpd->next_page != mpd->first_page) {
-		if (mpage_da_map_blocks(mpd) == 0)
-			mpage_da_submit_io(mpd);
-
-		mpd->io_done = 1;
-		ret = MPAGE_DA_EXTENT_TAIL;
-	}
-	wbc->nr_to_write -= mpd->pages_written;
-	return ret;
-}
-
-/*
  * this is a special callback for ->write_begin() only
  * it's intention is to return mapped block or reserve space
  */
@@ -2541,7 +2500,38 @@ retry:
 			dump_stack();
 			goto out_writepages;
 		}
-		ret = mpage_da_writepages(mapping, wbc, &mpd);
+
+		/*
+		 * Now call __mpage_da_writepage to find the next
+		 * contiguous region of logical blocks that need
+		 * blocks to be allocated by ext4.  We don't actually
+		 * submit the blocks for I/O here, even though
+		 * write_cache_pages thinks it will, and will set the
+		 * pages as clean for write before calling
+		 * __mpage_da_writepage().
+		 */
+		mpd.b_size = 0;
+		mpd.b_state = 0;
+		mpd.b_blocknr = 0;
+		mpd.first_page = 0;
+		mpd.next_page = 0;
+		mpd.io_done = 0;
+		mpd.pages_written = 0;
+		mpd.retval = 0;
+		ret = write_cache_pages(mapping, wbc, __mpage_da_writepage,
+					&mpd);
+		/*
+		 * If we have a contigous extent of pages and we
+		 * haven't done the I/O yet, map the blocks and submit
+		 * them for I/O.
+		 */
+		if (!mpd.io_done && mpd.next_page != mpd.first_page) {
+			if (mpage_da_map_blocks(&mpd) == 0)
+				mpage_da_submit_io(&mpd);
+			mpd.io_done = 1;
+			ret = MPAGE_DA_EXTENT_TAIL;
+		}
+		wbc->nr_to_write -= mpd.pages_written;
 
 		ext4_journal_stop(handle);
 
-- 
1.5.6.3

[PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Theodore Ts'o]

Add an ioctl which forces all of the delay allocated blocks to be
allocated.  This also provides a function ext4_alloc_da_blocks() which
will be used by the following commits to force files to be fully
allocated to preserve application-expected ext3 behaviour.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/ext4.h  |    2 ++
 fs/ext4/inode.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/ioctl.c |   13 +++++++++++++
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9840cad..626bda7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -316,6 +316,7 @@ struct ext4_new_group_data {
 #define EXT4_IOC_GROUP_EXTEND		_IOW('f', 7, unsigned long)
 #define EXT4_IOC_GROUP_ADD		_IOW('f', 8, struct ext4_new_group_input)
 #define EXT4_IOC_MIGRATE		_IO('f', 9)
+#define EXT4_IOC_ALLOC_DA_BLKS		_IO('f', 10)
  /* note ioctl 11 reserved for filesystem-independent FIEMAP ioctl */
 
 /*
@@ -1094,6 +1095,7 @@ extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
 extern int ext4_block_truncate_page(handle_t *handle,
 		struct address_space *mapping, loff_t from);
 extern int ext4_page_mkwrite(struct vm_area_struct *vma, struct page *page);
+extern int ext4_alloc_da_blocks(struct inode *inode);
 
 /* ioctl.c */
 extern long ext4_ioctl(struct file *, unsigned int, unsigned long);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1216fb9..c66af1c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2807,6 +2807,48 @@ out:
 	return;
 }
 
+/*
+ * Force all delayed allocation blocks to be allocated for a given inode.
+ */
+int ext4_alloc_da_blocks(struct inode *inode)
+{
+	if (!EXT4_I(inode)->i_reserved_data_blocks &&
+	    !EXT4_I(inode)->i_reserved_meta_blocks)
+		return 0;
+
+	/*
+	 * We do something simple for now.  The filemap_flush() will
+	 * also start triggering a write of the data blocks, which is
+	 * not strictly speaking necessary (and for users of
+	 * laptop_mode, not even desirable).  However, to do otherwise
+	 * would require replicating code paths in:
+	 * 
+	 * ext4_da_writepages() ->
+	 *    write_cache_pages() ---> (via passed in callback function)
+	 *        __mpage_da_writepage() -->
+	 *           mpage_add_bh_to_extent()
+	 *           mpage_da_map_blocks()
+	 *
+	 * The problem is that write_cache_pages(), located in
+	 * mm/page-writeback.c, marks pages clean in preparation for
+	 * doing I/O, which is not desirable if we're not planning on
+	 * doing I/O at all.
+	 *
+	 * We could call write_cache_pages(), and then redirty all of
+	 * the pages by calling redirty_page_for_writeback() but that
+	 * would be ugly in the extreme.  So instead we would need to
+	 * replicate parts of the code in the above functions,
+	 * simplifying them becuase we wouldn't actually intend to
+	 * write out the pages, but rather only collect contiguous
+	 * logical block extents, call the multi-block allocator, and
+	 * then update the buffer heads with the block allocations.
+	 * 
+	 * For now, though, we'll cheat by calling filemap_flush(),
+	 * which will map the blocks, and start the I/O, but not
+	 * actually wait for the I/O to complete.
+	 */
+	return filemap_flush(inode->i_mapping);
+}
 
 /*
  * bmap() is special.  It gets used by applications such as lilo and by
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 22dd29f..0d59f5f 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -262,6 +262,19 @@ setversion_out:
 		return err;
 	}
 
+	case EXT4_IOC_ALLOC_DA_BLKS:
+	{
+		int err;
+		if (!is_owner_or_cap(inode))
+			return -EACCES;
+
+		err = mnt_want_write(filp->f_path.mnt);
+		if (err)
+			return err;
+		err = ext4_alloc_da_blocks(inode);
+		return err;
+	}
+
 	default:
 		return -ENOTTY;
 	}
-- 
1.5.6.3

[PATCH, RFC] ext4: Automatically allocate delay allocated blocks on close

[Theodore Ts'o]

When closing a file that had been previously truncated, force any
delay allocated blocks that to be allocated so that if the filesystem
is mounted with data=ordered, the data blocks will be pushed out to
disk along with the journal commit.  Many application programs expect
this, so we do this to avoid zero length files if the system crashes
unexpectedly.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/ext4.h  |    1 +
 fs/ext4/file.c  |    4 ++++
 fs/ext4/inode.c |    2 ++
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 626bda7..b56245d 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -269,6 +269,7 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
 #define EXT4_STATE_NEW			0x00000002 /* inode is newly created */
 #define EXT4_STATE_XATTR		0x00000004 /* has in-inode xattrs */
 #define EXT4_STATE_NO_EXPAND		0x00000008 /* No space for expansion */
+#define EXT4_STATE_DA_ALLOC_CLOSE	0x00000010 /* Alloc DA blks on close */
 
 /* Used to pass group descriptor data when online resize is done */
 struct ext4_new_group_input {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index f731cb5..06df827 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -33,6 +33,10 @@
  */
 static int ext4_release_file(struct inode *inode, struct file *filp)
 {
+	if (EXT4_I(inode)->i_state & EXT4_STATE_DA_ALLOC_CLOSE) {
+		ext4_alloc_da_blocks(inode);
+		EXT4_I(inode)->i_state &= ~EXT4_STATE_DA_ALLOC_CLOSE;
+	}
 	/* if we are the last writer on the inode, drop the block reservation */
 	if ((filp->f_mode & FMODE_WRITE) &&
 			(atomic_read(&inode->i_writecount) == 1))
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c66af1c..22993ca 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3871,6 +3871,8 @@ void ext4_truncate(struct inode *inode)
 	if (!ext4_can_truncate(inode))
 		return;
 
+	ei->i_state |= EXT4_STATE_DA_ALLOC_CLOSE;
+
 	if (EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL) {
 		ext4_ext_truncate(inode);
 		return;
-- 
1.5.6.3

[PATCH, RFC] ext4: Automatically allocate delay allocated blocks on rename

[Theodore Ts'o]

When renaming a file such that a link to another inode is overwritten,
force any delay allocated blocks that to be allocated so that if the
filesystem is mounted with data=ordered, the data blocks will be
pushed out to disk along with the journal commit.  Many application
programs expect this, so we do this to avoid zero length files if the
system crashes unexpectedly.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/namei.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index cb15900..a9a7581 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2357,7 +2357,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct inode *old_inode, *new_inode;
 	struct buffer_head *old_bh, *new_bh, *dir_bh;
 	struct ext4_dir_entry_2 *old_de, *new_de;
-	int retval;
+	int retval, force_da_alloc = 0;
 
 	old_bh = new_bh = dir_bh = NULL;
 
@@ -2497,6 +2497,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 		ext4_mark_inode_dirty(handle, new_inode);
 		if (!new_inode->i_nlink)
 			ext4_orphan_add(handle, new_inode);
+		force_da_alloc = 1;
 	}
 	retval = 0;
 
@@ -2505,6 +2506,8 @@ end_rename:
 	brelse(old_bh);
 	brelse(new_bh);
 	ext4_journal_stop(handle);
+	if (retval == 0 && force_da_alloc)
+		ext4_alloc_da_blocks(old_inode);
 	return retval;
 }
 
-- 
1.5.6.3

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Aneesh Kumar K.V]

On Tue, Feb 24, 2009 at 12:05:50AM -0500, Theodore Ts'o wrote:
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 22dd29f..0d59f5f 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -262,6 +262,19 @@ setversion_out:
>  		return err;
>  	}
> 
> +	case EXT4_IOC_ALLOC_DA_BLKS:
> +	{
> +		int err;
> +		if (!is_owner_or_cap(inode))
> +			return -EACCES;
> +
> +		err = mnt_want_write(filp->f_path.mnt);
> +		if (err)
> +			return err;
> +		err = ext4_alloc_da_blocks(inode);
> +		return err;
> +	}
> +

I guess we need a  matching
	mnt_drop_write();

>  	default:
>  		return -ENOTTY;
>  	}
> -- 
> 1.5.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Andreas Dilger]

On Feb 24, 2009  00:05 -0500, Theodore Ts'o wrote:
> +#define EXT4_IOC_ALLOC_DA_BLKS		_IO('f', 10)
>   /* note ioctl 11 reserved for filesystem-independent FIEMAP ioctl */

Please don't use ioctl number 10 - it was used by an early
version of the FIEMAP ioctl that we still have in use in Lustre
filesystems.  Not that there is much chance of a collision, but
it is also easily enough avoided.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH, RFC] ext4: Automatically allocate delay allocated blocks on close

[Andreas Dilger]

On Feb 24, 2009  00:05 -0500, Theodore Ts'o wrote:
> When closing a file that had been previously truncated, force any
> delay allocated blocks that to be allocated so that if the filesystem
> is mounted with data=ordered, the data blocks will be pushed out to
> disk along with the journal commit.  Many application programs expect
> this, so we do this to avoid zero length files if the system crashes
> unexpectedly.

Should this only be done with "truncate-to-zero" operations, or any
truncate?  Some applications may do extending truncates in order to
trigger file preallocation ala Windows, and we don't necessarily want
to punish all of the IO for those files.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Theodore Tso]

On Tue, Feb 24, 2009 at 03:08:32PM +0530, Aneesh Kumar K.V wrote:
> 
> I guess we need a  matching
> 	mnt_drop_write();
> 

Oops.  Nice catch, thanks.  I'll fixed it up in the ext4 patch queue.

       	    	   	    	       	     - Ted

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Theodore Tso]

On Tue, Feb 24, 2009 at 03:11:05AM -0700, Andreas Dilger wrote:
> On Feb 24, 2009  00:05 -0500, Theodore Ts'o wrote:
> > +#define EXT4_IOC_ALLOC_DA_BLKS		_IO('f', 10)
> >   /* note ioctl 11 reserved for filesystem-independent FIEMAP ioctl */
> 
> Please don't use ioctl number 10 - it was used by an early
> version of the FIEMAP ioctl that we still have in use in Lustre
> filesystems.  Not that there is much chance of a collision, but
> it is also easily enough avoided.

Ok, I'll also add a note that ioctl number 10 should be reserved.

    	      	    	      	    	   - Ted

Re: [PATCH, RFC] ext4: Automatically allocate delay allocated blocks on close

[Theodore Tso]

On Tue, Feb 24, 2009 at 03:13:44AM -0700, Andreas Dilger wrote:
> On Feb 24, 2009  00:05 -0500, Theodore Ts'o wrote:
> > When closing a file that had been previously truncated, force any
> > delay allocated blocks that to be allocated so that if the filesystem
> > is mounted with data=ordered, the data blocks will be pushed out to
> > disk along with the journal commit.  Many application programs expect
> > this, so we do this to avoid zero length files if the system crashes
> > unexpectedly.
> 
> Should this only be done with "truncate-to-zero" operations, or any
> truncate?  Some applications may do extending truncates in order to
> trigger file preallocation ala Windows, and we don't necessarily want
> to punish all of the IO for those files.

Agreed, we should only do this on a truncate-to-zero.  I'll fix up the
patch to only set EXT4_STATE_DA_ALLOC_CLOSE on truncate if
inode->i_size is 0.

							- Ted

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Eric Sandeen]

Theodore Ts'o wrote:
> Add an ioctl which forces all of the delay allocated blocks to be
> allocated.  This also provides a function ext4_alloc_da_blocks() which
> will be used by the following commits to force files to be fully
> allocated to preserve application-expected ext3 behaviour.
> 

Is it worth checking whether a) the file has delalloc blocks, and/or b)
whether the mapping is dirty before we spin off a filemap_flush?

-Eric

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Aneesh Kumar K.V]

On Wed, Mar 11, 2009 at 05:41:05PM -0500, Eric Sandeen wrote:
> Theodore Ts'o wrote:
> > Add an ioctl which forces all of the delay allocated blocks to be
> > allocated.  This also provides a function ext4_alloc_da_blocks() which
> > will be used by the following commits to force files to be fully
> > allocated to preserve application-expected ext3 behaviour.
> > 
> 
> Is it worth checking whether a) the file has delalloc blocks, and/or b)
> whether the mapping is dirty before we spin off a filemap_flush?
> 
+int ext4_alloc_da_blocks(struct inode *inode)
+{
+       if (!EXT4_I(inode)->i_reserved_data_blocks &&
+           !EXT4_I(inode)->i_reserved_meta_blocks)
+               return 0;

This check test does (a). Since the ioctl is to force allocation of
delayed allocated blocks i guess (a) is enough because we don't want to
cause a filemap_flush when we don't have any delayed allocated blocks
but have dirty pages around.

-aneesh

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Eric Sandeen]

Aneesh Kumar K.V wrote:
> On Wed, Mar 11, 2009 at 05:41:05PM -0500, Eric Sandeen wrote:
>> Theodore Ts'o wrote:
>>> Add an ioctl which forces all of the delay allocated blocks to be
>>> allocated.  This also provides a function ext4_alloc_da_blocks() which
>>> will be used by the following commits to force files to be fully
>>> allocated to preserve application-expected ext3 behaviour.
>>>
>> Is it worth checking whether a) the file has delalloc blocks, and/or b)
>> whether the mapping is dirty before we spin off a filemap_flush?
>>
> +int ext4_alloc_da_blocks(struct inode *inode)
> +{
> +       if (!EXT4_I(inode)->i_reserved_data_blocks &&
> +           !EXT4_I(inode)->i_reserved_meta_blocks)
> +               return 0;
> 
> This check test does (a). Since the ioctl is to force allocation of
> delayed allocated blocks i guess (a) is enough because we don't want to
> cause a filemap_flush when we don't have any delayed allocated blocks
> but have dirty pages around.

and b) is as simple as

if (!mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY))
	return 0;

I think?

-Eric

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Theodore Tso]

On Thu, Mar 12, 2009 at 11:32:18AM -0500, Eric Sandeen wrote:
> Aneesh Kumar K.V wrote:
> > On Wed, Mar 11, 2009 at 05:41:05PM -0500, Eric Sandeen wrote:
> >> Theodore Ts'o wrote:
> >>> Add an ioctl which forces all of the delay allocated blocks to be
> >>> allocated.  This also provides a function ext4_alloc_da_blocks() which
> >>> will be used by the following commits to force files to be fully
> >>> allocated to preserve application-expected ext3 behaviour.
> >>>
> >> Is it worth checking whether a) the file has delalloc blocks, and/or b)
> >> whether the mapping is dirty before we spin off a filemap_flush?
> >>
> > +int ext4_alloc_da_blocks(struct inode *inode)
> > +{
> > +       if (!EXT4_I(inode)->i_reserved_data_blocks &&
> > +           !EXT4_I(inode)->i_reserved_meta_blocks)
> > +               return 0;
> > 
> > This check test does (a). Since the ioctl is to force allocation of
> > delayed allocated blocks i guess (a) is enough because we don't want to
> > cause a filemap_flush when we don't have any delayed allocated blocks
> > but have dirty pages around.
> 
> and b) is as simple as
> 
> if (!mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY))
> 	return 0;

Yeah, but (b) isn't necessary; if there are some delayed allocation
blocks, by definition there must be some dirty pages, right?

	   	      	    	    	 - Ted

Re: [PATCH, RFC] ext4: add EXT4_IOC_ALLOC_DA_BLKS ioctl

[Eric Sandeen]

Theodore Tso wrote:
> On Thu, Mar 12, 2009 at 11:32:18AM -0500, Eric Sandeen wrote:
>> Aneesh Kumar K.V wrote:
>>> On Wed, Mar 11, 2009 at 05:41:05PM -0500, Eric Sandeen wrote:
>>>> Theodore Ts'o wrote:
>>>>> Add an ioctl which forces all of the delay allocated blocks to be
>>>>> allocated.  This also provides a function ext4_alloc_da_blocks() which
>>>>> will be used by the following commits to force files to be fully
>>>>> allocated to preserve application-expected ext3 behaviour.
>>>>>
>>>> Is it worth checking whether a) the file has delalloc blocks, and/or b)
>>>> whether the mapping is dirty before we spin off a filemap_flush?
>>>>
>>> +int ext4_alloc_da_blocks(struct inode *inode)
>>> +{
>>> +       if (!EXT4_I(inode)->i_reserved_data_blocks &&
>>> +           !EXT4_I(inode)->i_reserved_meta_blocks)
>>> +               return 0;
>>>
>>> This check test does (a). Since the ioctl is to force allocation of
>>> delayed allocated blocks i guess (a) is enough because we don't want to
>>> cause a filemap_flush when we don't have any delayed allocated blocks
>>> but have dirty pages around.
>> and b) is as simple as
>>
>> if (!mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY))
>> 	return 0;
> 
> Yeah, but (b) isn't necessary; if there are some delayed allocation
> blocks, by definition there must be some dirty pages, right?
> 
> 	   	      	    	    	 - Ted

oh, heh.  Right you are :)

-Eric