[patch 4/4] ext4: replace __FUNCTION__ occurrences

[patch 4/4] ext4: replace __FUNCTION__ occurrences

[akpm]

From: "Stoyan Gaydarov" <stoyboyker@gmail.com>

__FUNCTION__ is gcc-specific, use __func__ instead

Signed-off-by: Stoyan Gaydarov <stoyboyker@gmail.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Mingming Cao <cmm@us.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/ext4/ext4.h      |    4 ++--
 fs/ext4/ext4_jbd2.h |   14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff -puN fs/ext4/ext4.h~ext4-replace-__function__-occurrences fs/ext4/ext4.h
--- a/fs/ext4/ext4.h~ext4-replace-__function__-occurrences
+++ a/fs/ext4/ext4.h
@@ -45,7 +45,7 @@
 #define ext4_debug(f, a...)						\
 	do {								\
 		printk (KERN_DEBUG "EXT4-fs DEBUG (%s, %d): %s:",	\
-			__FILE__, __LINE__, __FUNCTION__);		\
+			__FILE__, __LINE__, __func__);			\
 		printk (KERN_DEBUG f, ## a);				\
 	} while (0)
 #else
@@ -1203,7 +1203,7 @@ static inline unsigned int ext4_flex_bg_
 #define ext4_std_error(sb, errno)				\
 do {								\
 	if ((errno))						\
-		__ext4_std_error((sb), __FUNCTION__, (errno));	\
+		__ext4_std_error((sb), __func__, (errno));	\
 } while (0)
 
 /*
diff -puN fs/ext4/ext4_jbd2.h~ext4-replace-__function__-occurrences fs/ext4/ext4_jbd2.h
--- a/fs/ext4/ext4_jbd2.h~ext4-replace-__function__-occurrences
+++ a/fs/ext4/ext4_jbd2.h
@@ -142,17 +142,17 @@ int __ext4_journal_dirty_metadata(const 
 				handle_t *handle, struct buffer_head *bh);
 
 #define ext4_journal_get_undo_access(handle, bh) \
-	__ext4_journal_get_undo_access(__FUNCTION__, (handle), (bh))
+	__ext4_journal_get_undo_access(__func__, (handle), (bh))
 #define ext4_journal_get_write_access(handle, bh) \
-	__ext4_journal_get_write_access(__FUNCTION__, (handle), (bh))
+	__ext4_journal_get_write_access(__func__, (handle), (bh))
 #define ext4_journal_revoke(handle, blocknr, bh) \
-	__ext4_journal_revoke(__FUNCTION__, (handle), (blocknr), (bh))
+	__ext4_journal_revoke(__func__, (handle), (blocknr), (bh))
 #define ext4_journal_get_create_access(handle, bh) \
-	__ext4_journal_get_create_access(__FUNCTION__, (handle), (bh))
+	__ext4_journal_get_create_access(__func__, (handle), (bh))
 #define ext4_journal_dirty_metadata(handle, bh) \
-	__ext4_journal_dirty_metadata(__FUNCTION__, (handle), (bh))
+	__ext4_journal_dirty_metadata(__func__, (handle), (bh))
 #define ext4_journal_forget(handle, bh) \
-	__ext4_journal_forget(__FUNCTION__, (handle), (bh))
+	__ext4_journal_forget(__func__, (handle), (bh))
 
 handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks);
 int __ext4_journal_stop(const char *where, handle_t *handle);
@@ -163,7 +163,7 @@ static inline handle_t *ext4_journal_sta
 }
 
 #define ext4_journal_stop(handle) \
-	__ext4_journal_stop(__FUNCTION__, (handle))
+	__ext4_journal_stop(__func__, (handle))
 
 static inline handle_t *ext4_journal_current_handle(void)
 {
_

Re: [patch 4/4] ext4: replace __FUNCTION__ occurrences

[Theodore Tso]

On Sat, Jul 12, 2008 at 11:26:21PM -0700, akpm@linux-foundation.org wrote:
> From: "Stoyan Gaydarov" <stoyboyker@gmail.com>
> 
> __FUNCTION__ is gcc-specific, use __func__ instead
> 

Thanks, applied.

						- Ted

[PATCH] Ext4: Fix delalloc enospace handling counter update race

[Mingming Cao]

Ext4: Fix delalloc enospace handling counter update race

From: Mingming Cao <cmm@us.ibm.com>

Ext4 delalloc reserve meta blocks ahead of time so later when real block
allocation fs will not short of free blocks for allocating meta blocks.
It keeps track of the real number of new allocated meta data blocks, so
after block allocation it will update how much meta data blocks still
need to be reserved for that inode.

Both per inode reserved metadata blocks and per-allocation allocated metablocks
are protected by the per inode delalloc reservation lock. The per-allocation
allocated metablocks counter should be protected by the i_data_sem as well,
so that it could avoid race with other block allocation to the same inode
in parallel. The patch moves the code under the i_data_sem protection.

Also in the case of truncate, we should not clear the per-allocation allocated
metablocks counter as that may be in-use by parallel allocation. The patch
only clear the per-allocation allocated metablocks when allocation is successfully returned.


Signed-off-by: Mingming Cao <cmm@us.ibm.com>

---
 fs/ext4/inode.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Index: linux-2.6.26-rc9/fs/ext4/inode.c
===================================================================
--- linux-2.6.26-rc9.orig/fs/ext4/inode.c	2008-07-14 08:54:25.000000000 -0700
+++ linux-2.6.26-rc9/fs/ext4/inode.c	2008-07-14 09:23:29.000000000 -0700
@@ -1060,8 +1060,18 @@ int ext4_get_blocks_wrap(handle_t *handl
 							~EXT4_EXT_MIGRATE;
 		}
 	}
-	if (flag)
+
+	if (flag) {
 		EXT4_I(inode)->i_delalloc_reserved_flag = 0;
+		/*
+		 * Update reserved blocks/metadata blocks
+		 * after successful block allocation
+		 * which were deferred till now
+		 */
+		if ((retval > 0) && buffer_delay(bh))
+			ext4_da_release_space(inode, retval, 0);
+	}
+
 	up_write((&EXT4_I(inode)->i_data_sem));
 	return retval;
 }
@@ -1519,7 +1529,8 @@ void ext4_da_release_space(struct inode 
 
 	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
 	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
-	EXT4_I(inode)->i_allocated_meta_blocks = 0;
+	if (used)
+		EXT4_I(inode)->i_allocated_meta_blocks = 0;
 	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
 }
 
@@ -2005,10 +2016,6 @@ static int ext4_da_get_block_write(struc
 	if (ret > 0) {
 		bh_result->b_size = (ret << inode->i_blkbits);
 
-		/* release reserved-but-unused meta blocks */
-		if (buffer_delay(bh_result))
-			ext4_da_release_space(inode, ret, 0);

Re: [PATCH] Ext4: Fix delalloc enospace handling counter update race

[Aneesh Kumar K.V]

On Mon, Jul 14, 2008 at 09:28:17AM -0700, Mingming Cao wrote:
> 
> Ext4: Fix delalloc enospace handling counter update race
> 
> From: Mingming Cao <cmm@us.ibm.com>
> 
> Ext4 delalloc reserve meta blocks ahead of time so later when real block
> allocation fs will not short of free blocks for allocating meta blocks.
> It keeps track of the real number of new allocated meta data blocks, so
> after block allocation it will update how much meta data blocks still
> need to be reserved for that inode.
> 
> Both per inode reserved metadata blocks and per-allocation allocated metablocks
> are protected by the per inode delalloc reservation lock. The per-allocation
> allocated metablocks counter should be protected by the i_data_sem as well,
> so that it could avoid race with other block allocation to the same inode
> in parallel. The patch moves the code under the i_data_sem protection.
> 
> Also in the case of truncate, we should not clear the per-allocation allocated
> metablocks counter as that may be in-use by parallel allocation. The patch
> only clear the per-allocation allocated metablocks when allocation is successfully returned.
> 
> 
> Signed-off-by: Mingming Cao <cmm@us.ibm.com>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> 
> ---
>  fs/ext4/inode.c |   19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6.26-rc9/fs/ext4/inode.c
> ===================================================================
> --- linux-2.6.26-rc9.orig/fs/ext4/inode.c	2008-07-14 08:54:25.000000000 -0700
> +++ linux-2.6.26-rc9/fs/ext4/inode.c	2008-07-14 09:23:29.000000000 -0700
> @@ -1060,8 +1060,18 @@ int ext4_get_blocks_wrap(handle_t *handl
>  							~EXT4_EXT_MIGRATE;
>  		}
>  	}
> -	if (flag)
> +
> +	if (flag) {
>  		EXT4_I(inode)->i_delalloc_reserved_flag = 0;
> +		/*
> +		 * Update reserved blocks/metadata blocks
> +		 * after successful block allocation
> +		 * which were deferred till now
> +		 */
> +		if ((retval > 0) && buffer_delay(bh))
> +			ext4_da_release_space(inode, retval, 0);
> +	}
> +
>  	up_write((&EXT4_I(inode)->i_data_sem));
>  	return retval;
>  }
> @@ -1519,7 +1529,8 @@ void ext4_da_release_space(struct inode 
> 
>  	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
>  	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
> -	EXT4_I(inode)->i_allocated_meta_blocks = 0;
> +	if (used)
> +		EXT4_I(inode)->i_allocated_meta_blocks = 0;
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  }
> 
> @@ -2005,10 +2016,6 @@ static int ext4_da_get_block_write(struc
>  	if (ret > 0) {
>  		bh_result->b_size = (ret << inode->i_blkbits);
> 
> -		/* release reserved-but-unused meta blocks */
> -		if (buffer_delay(bh_result))
> -			ext4_da_release_space(inode, ret, 0);
> -
>  		/*
>  		 * Update on-disk size along with block allocation
>  		 * we don't use 'extend_disksize' as size may change
> 
> 

Re: [PATCH] Ext4: Fix delalloc enospace handling counter update race

[Mingming Cao]

在 2008-07-14一的 22:23 +0530，Aneesh Kumar K.V写道：
> On Mon, Jul 14, 2008 at 09:28:17AM -0700, Mingming Cao wrote:
> > 
> > Ext4: Fix delalloc enospace handling counter update race
> > 
> > From: Mingming Cao <cmm@us.ibm.com>

> > Also in the case of truncate, we should not clear the per-allocation allocated
> > metablocks counter as that may be in-use by parallel allocation. The patch
> > only clear the per-allocation allocated metablocks when allocation is successfully returned.
> > 
> > 
> > Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> 
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>


> > @@ -1519,7 +1529,8 @@ void ext4_da_release_space(struct inode 
> > 
> >  	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> >  	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
> > -	EXT4_I(inode)->i_allocated_meta_blocks = 0;
> > +	if (used)
> > +		EXT4_I(inode)->i_allocated_meta_blocks = 0;
> >  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> >  }
> > 

After think again, this part of race fix may not needed, when the first
part of patch is applied. Since truncate is also hold the i_data_sem, so
it could be assured that when truncate is releasing the page's  reserved
blocks, there is no parallel block allocation.

Updated patch with only the first race fix, 

If no objection I will fold this to the parent patch
delalloc-ext4-ENOSPC-handling.patch in the patch queue.

Regards,
Mingming

Ext4: Fix delalloc enospace handling counter update race

From: Mingming Cao <cmm@us.ibm.com>

Ext4 delalloc reserve meta blocks ahead of time so later when real block
allocation fs will not short of free blocks for allocating meta blocks.
It keeps track of the real number of new allocated meta data blocks, so
after block allocation it will update how much meta data blocks still
need to be reserved for that inode.

Both per inode reserved metadata blocks and per-allocation allocated metablocks
are protected by the per inode delalloc reservation lock. The per-allocation
allocated metablocks counter should be protected by the i_data_sem as well,
so that it could avoid race with other block allocation to the same inode
in parallel. The patch moves the code under the i_data_sem protection.

Signed-off-by: Mingming Cao <cmm@us.ibm.com>

---
 fs/ext4/inode.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Index: linux-2.6.26-rc9/fs/ext4/inode.c
===================================================================
--- linux-2.6.26-rc9.orig/fs/ext4/inode.c	2008-07-14 08:54:25.000000000 -0700
+++ linux-2.6.26-rc9/fs/ext4/inode.c	2008-07-14 10:51:00.000000000 -0700
@@ -1060,8 +1060,18 @@ int ext4_get_blocks_wrap(handle_t *handl
 							~EXT4_EXT_MIGRATE;
 		}
 	}
-	if (flag)
+
+	if (flag) {
 		EXT4_I(inode)->i_delalloc_reserved_flag = 0;
+		/*
+		 * Update reserved blocks/metadata blocks
+		 * after successful block allocation
+		 * which were deferred till now
+		 */
+		if ((retval > 0) && buffer_delay(bh))
+			ext4_da_release_space(inode, retval, 0);
+	}
+
 	up_write((&EXT4_I(inode)->i_data_sem));
 	return retval;
 }
@@ -2005,10 +2015,6 @@ static int ext4_da_get_block_write(struc
 	if (ret > 0) {
 		bh_result->b_size = (ret << inode->i_blkbits);
 
-		/* release reserved-but-unused meta blocks */
-		if (buffer_delay(bh_result))
-			ext4_da_release_space(inode, ret, 0);

Re: [PATCH] Ext4: Fix delalloc enospace handling counter update race

[Theodore Tso]

On Mon, Jul 14, 2008 at 10:54:36AM -0700, Mingming Cao wrote:
> Updated patch with only the first race fix, 
> 
> If no objection I will fold this to the parent patch
> delalloc-ext4-ENOSPC-handling.patch in the patch queue.

If you're going to fold this to the parent patch (which makes sense),
the following grammatical correction won't be necessary, but here are
some mostly verb tense fixups.

> Ext4: Fix delalloc enospace handling counter update race
> 
> From: Mingming Cao <cmm@us.ibm.com>
> 
> Ext4 delalloc reserve meta blocks ahead of time so later when real block
                reserves
> allocation fs will not short of free blocks for allocating meta blocks.
  	        "will not run out"
> It keeps track of the real number of new allocated meta data blocks, so
     	   	    	     	       newly                          and
> after block allocation it will update how much meta data blocks still
                            updates
> need to be reserved for that inode.
> 
> Both per inode reserved metadata blocks and per-allocation allocated metablocks
> are protected by the per inode delalloc reservation lock. The per-allocation
> allocated metablocks counter should be protected by the i_data_sem as well,
> so that it could avoid race with other block allocation to the same inode
  to avoid racing with ther block allocations
> in parallel. The patch moves the code under the i_data_sem protection.

Please feel free to add a

       Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

to the patch.

     	       	   	       	   	      - Ted

Re: [PATCH] Ext4: Fix delalloc enospace handling counter update race

[Mingming Cao]

在 2008-07-14一的 14:05 -0400，Theodore Tso写道：
> On Mon, Jul 14, 2008 at 10:54:36AM -0700, Mingming Cao wrote:
> > Updated patch with only the first race fix, 
> > 
> > If no objection I will fold this to the parent patch
> > delalloc-ext4-ENOSPC-handling.patch in the patch queue.
> 
> If you're going to fold this to the parent patch (which makes sense),
> the following grammatical correction won't be necessary, but here are
> some mostly verb tense fixups.
> 

Thanks for going through the grammers and fixed the errors for me.

> Please feel free to add a
> 
>        Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> 
> to the patch.
> 

I added your Signed-off to the patch
delalloc-ext4-ENOSPC-handling.patch

Regrads,

Mingming
>      	       	   	       	   	      - Ted
> --
> 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

--
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] Ext4: Fix delalloc enospace handling counter update race

[Aneesh Kumar K.V]

On Mon, Jul 14, 2008 at 10:54:36AM -0700, Mingming Cao wrote:
> 
> 在 2008-07-14一的 22:23 +0530，Aneesh Kumar K.V写道：
> > On Mon, Jul 14, 2008 at 09:28:17AM -0700, Mingming Cao wrote:
> > > 
> > > Ext4: Fix delalloc enospace handling counter update race
> > > 
> > > From: Mingming Cao <cmm@us.ibm.com>
> 
> > > Also in the case of truncate, we should not clear the per-allocation allocated
> > > metablocks counter as that may be in-use by parallel allocation. The patch
> > > only clear the per-allocation allocated metablocks when allocation is successfully returned.
> > > 
> > > 
> > > Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> > 
> > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> 
> > > @@ -1519,7 +1529,8 @@ void ext4_da_release_space(struct inode 
> > > 
> > >  	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> > >  	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
> > > -	EXT4_I(inode)->i_allocated_meta_blocks = 0;
> > > +	if (used)
> > > +		EXT4_I(inode)->i_allocated_meta_blocks = 0;
> > >  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> > >  }
> > > 
> 
> After think again, this part of race fix may not needed, when the first
> part of patch is applied. Since truncate is also hold the i_data_sem, so
> it could be assured that when truncate is releasing the page's  reserved
> blocks, there is no parallel block allocation.

But truncate calls invalidate_page without holding i_data_sem.

vmtruncate -> truncate_inode_pages -> invalidate_page.


-aneesh
--
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] Ext4: Fix delalloc enospace handling counter update race

[Mingming Cao]

在 2008-07-15二的 08:42 +0530，Aneesh Kumar K.V写道：
> On Mon, Jul 14, 2008 at 10:54:36AM -0700, Mingming Cao wrote:
> > 
> > 在 2008-07-14一的 22:23 +0530，Aneesh Kumar K.V写道：
> > > On Mon, Jul 14, 2008 at 09:28:17AM -0700, Mingming Cao wrote:
> > > > 
> > > > Ext4: Fix delalloc enospace handling counter update race
> > > > 
> > > > From: Mingming Cao <cmm@us.ibm.com>
> > 
> > > > Also in the case of truncate, we should not clear the per-allocation allocated
> > > > metablocks counter as that may be in-use by parallel allocation. The patch
> > > > only clear the per-allocation allocated metablocks when allocation is successfully returned.
> > > > 
> > > > 
> > > > Signed-off-by: Mingming Cao <cmm@us.ibm.com>
> > > 
> > > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > 
> > 
> > > > @@ -1519,7 +1529,8 @@ void ext4_da_release_space(struct inode 
> > > > 
> > > >  	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> > > >  	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
> > > > -	EXT4_I(inode)->i_allocated_meta_blocks = 0;
> > > > +	if (used)
> > > > +		EXT4_I(inode)->i_allocated_meta_blocks = 0;
> > > >  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> > > >  }
> > > > 
> > 
> > After think again, this part of race fix may not needed, when the first
> > part of patch is applied. Since truncate is also hold the i_data_sem, so
> > it could be assured that when truncate is releasing the page's  reserved
> > blocks, there is no parallel block allocation.
> 
> But truncate calls invalidate_page without holding i_data_sem.
> 
> vmtruncate -> truncate_inode_pages -> invalidate_page.
> 

Just for the benefit of the readers on the list, we have discussed this
on IRC, the race between truncate and parallel allocation is probably
not cause incorrect free blocks accounting,  but the code is confusing
in the current way. So it's worth a seperate cleanup patch later.

Mingming

--
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